hierynomus / smbj

Server Message Block (SMB2, SMB3) implementation in Java
Other
705 stars 179 forks source link

Spurious failures for cached DiskShares #790

Closed laeubi closed 11 months ago

laeubi commented 12 months ago

I'm using the SMBJ lib for quite a while but getting spurious errors, from the logfile it is like this:

[Packet Reader for <thehostname>] DEBUG com.hierynomus.smbj.transport.tcp.direct.DirectTcpPacketReader - Received packet Encrypted for session id << 400222232510481 >>
[Packet Reader for <thehostname>] DEBUG com.hierynomus.smbj.connection.packet.SMB3DecryptingPacketHandler - Decrypting packet Encrypted for session id << 400222232510481 >>
[Packet Reader for <thehostname>] DEBUG com.hierynomus.smbj.connection.packet.SMB3DecryptingPacketHandler - Decrypted packet Encrypted for session id << 400222232510481 >> is packet SMB2_TREE_DISCONNECT with message id << 114 >>.
[Packet Reader for <thehostname>] DEBUG com.hierynomus.smbj.connection.packet.SMB2SignatureVerificationPacketHandler - Passthrough Signature Verification as packet is decrypted
[Packet Reader for <thehostname>] DEBUG com.hierynomus.smbj.connection.packet.SMB2CreditGrantingPacketHandler - Server granted us 2 credits for SMB2_TREE_DISCONNECT with message id << 114 >>, now available: 623 credits
[Packet Reader for <thehostname>] DEBUG com.hierynomus.protocol.commons.concurrent.Promise - Setting << 114 >> to `SMB2_TREE_DISCONNECT with message id << 114 >>`
[ForkJoinPool-1-worker-1] DEBUG com.hierynomus.smbj.session.Session - Notified of TreeDisconnected <<262189>>

<< in usercode i get the following exception >>
com.hierynomus.smbj.common.SMBRuntimeException: DiskShare has already been closed
    at com.hierynomus.smbj.share.Share.send(Share.java:364)
    at com.hierynomus.smbj.share.Share.sendReceive(Share.java:358)
    at com.hierynomus.smbj.share.Share.queryInfo(Share.java:184)
    at com.hierynomus.smbj.share.DiskShare.getFileInformation(DiskShare.java:310)
    at com.hierynomus.smbj.share.DiskEntry.getFileInformation(DiskEntry.java:85)
    at com.hierynomus.smbj.share.DiskShare.getFileInformation(DiskShare.java:296)
    at com.hierynomus.smbj.share.DiskShare.getFileInformation(DiskShare.java:288)

<<short after that>>
[ForkJoinPool-1-worker-1] INFO com.hierynomus.smbj.session.Session - Connecting to <thehostname> on session 400222232510481

so for me it seems the usercode is unlucky getting in the way while a reconnect is happening on the session.

It would be great if this can be detected that a reconnect is currently performed and the Share.send method is blocking until the reconnect is finished (either success or failed).

dkocher commented 12 months ago

This is possibly because the same DiskShare instance is shared between different callers (and closed by one of them) because of caching in TreeConnectTable when obtaining the share from Session#connectShare.

laeubi commented 12 months ago

@dkocher would it be possible to make the cache a ThreadLocal then? the Share implments AutoCloseable so I assume it should probably return the instance to the cache unclosed then instead of really closing it until the usecount == 0 ?

I think its quite impossible to control what others do with the disk share and when close is actually called...

dkocher commented 12 months ago

@laeubi I am not the maintainer of the library but came across the same issue and worked around it using synchronization ^1

laeubi commented 12 months ago

I think one problem is that still code can bypass that synchronization, what I'm wondering (maybe you have some experience) if then probably one better never close the share (and wait for the JVM exit instead?

dkocher commented 12 months ago

I think one problem is that still code can bypass that synchronization, what I'm wondering (maybe you have some experience) if then probably one better never close the share (and wait for the JVM exit instead?

It is probably intended by the library that callers use a single long living share. Not sure yet if it is a (performance) issue to have the disk share short lived and open and close frequently.

hierynomus commented 12 months ago

Actually it is even intended by the SMB spec... That was why it was built as such

laeubi commented 12 months ago

Actually it is even intended by the SMB spec... That was why it was built as such

You mean to open the session and never close it again? Can you explain what the AutoClosable was chosen here? Because most IDEs will complain if you not close an AutoClosable :-\

So to summarize, one would connect a share and then never touch that would be fine?

laeubi commented 11 months ago

I have now tested without closing the share and it seems to work quite fine, so I think the issue can be closed, maybe one should enhance the javadoc that a DiskShare (even though it implements AutoClosable) should usually not be closed unless the application shut down or it is made sure the share is not needed anymore.