[jcifs] Deadlock in SmbTransport

[Allen, Michael B (RSCH)]

Ok, I think I have the fix. Would someone be kind enough to try it for us? It's here:

    http://users.erols.com/mballen/jcifs/jcifs-0.7.6.dlfix.jar

What I did was basically just remove the SmbTree and SmbSession locks.
Instead I wrapped the contents of SmbTree.treeConnect/treeDisconnect and
SmbSession.sessionSetup/logoff with a lock on the common transport. I *think*
this should fix it because there will no longer be a lock on these objects. The lock
to protect that code is now the lock the thread already has on transport and
recursive locks are ok. If this works as I understand it, it should work quite well. It's
definately much simpler.

Let me know what your performance is like. Even though the locking isn't "fine
grained" it is only for these 4 operations. Other transactions will proceed as
normal. In fact by removing the locks on SmbTree and SmbSession it might even
be faster. I'm seeing very good interleaving of these four operations with
ThreadedSmbCrawler which is encouraging.

Mike

> -----Original Message-----
> From:	Konrad Kaminski [SMTP:konrad.kaminski at javart.com.pl]
> Sent:	Wednesday, May 21, 2003 4:44 AM
> To:	jcifs at lists.samba.org
> Subject:	[jcifs] Deadlock in SmbTransport
> 
> Hi!
>  
> I found a similar problem to the one which had been discussed in December (Thread: Deadlock in Java code <http://lists.samba.org/pipermail/jcifs/2002-December/001574.html>). The problem is that in
> SmbTransport.tryClose () method there's a loop over all of the sessions (SmbSession) associated with this SmbTransport. This method however can be called from
>  
> SmbTree.treeConnect (locks SmbTree AND SmbSession)
> |---- SmbSession.send
>        |------------- SmbSession.sessionSetup (locks SmbSession again)
>                         |------------------ SmbTransport.send
>                                                |----------------- SmbTransport.tryClose ()
>  
> So if say there's a SmbTree.treeConnect called from SmbSession_1 and concurrently from SmbSession_2, and if both SmbSession's are associated with the same SmbTransport, then if for any reason (eg.
> DC connection timeout) both will call SmbTransport.tryClose() then there's a deadlock :
>  
> THREAD_1 : time(0) : SmbSession_1 (LOCK SmbSession_1) 
> THREAD_2 : time(1) : SmbSession_2 (LOCK SmbSession_2)
> THREAD_1 : time(2) : SmbTransport.tryClose - iterates over SmbSession and tries to do SmbSession2.logoff (synchronized and LOCKS -> wait)
> THREAD_2 : time(3) : SmbTransport.tryClose - iterates over SmbSession and tries to do SmbSession1.logoff (synchronized and LOCKS -> wait forever -> DEADLOCK)
>  
> The simplest solution would be to do a some sort of synchronization before calling the SmbTree.treeConnect method, but that would make it very inefficient (if you need to call treeConnect very often
> - that's the situation in our project). Do you know of any other solution? BTW I think this is also a bug.
>  
> Regards, Konrad Kaminski