[jcifs] deadlock in jcifs jar..

[Michael B Allen]

On Sat, Sep 20, 2008 at 9:06 PM, Con O'Donnell <conod at hotmail.com> wrote:
> Hi I think there is a potential deadlock condition in 1.2.23/1.2.24
>
> If I call logon from mutliple threads and one of them expires:
>                                         /* logoff old sessions */
>         if (SO_TIMEOUT > 0 && sessionExpiration < (now =
> System.currentTimeMillis())) {
>             sessionExpiration = now + SO_TIMEOUT;
>             iter = sessions.listIterator();
>             while( iter.hasNext() ) {
>                 ssn = (SmbSession)iter.next();
>                 if( ssn.expiration < now ) {
>                     ssn.logoff( false );
>                 }
>             }
>
> then the session logoff is called...
>
>
> I then occassionally get a deadlock:
>
> "Thread-19":
>         at jcifs.smb.SmbTree.treeConnect(SmbTree.java:128)
>         - waiting to lock <0x1b9129a8> (a java.lang.Object)
>         at jcifs.smb.SmbTree.send(SmbTree.java:64)
>         at jcifs.smb.SmbTree.treeDisconnect(SmbTree.java:168)
>         - locked <0x1b90f2a0> (a jcifs.smb.SmbTransport)
>         at jcifs.smb.SmbSession.logoff(SmbSession.java:301)
>         - locked <0x1b90f2a0> (a jcifs.smb.SmbTransport)
>         at jcifs.smb.SmbTransport.getSmbSession(SmbTransport.java:138)
>         - locked <0x1b90f2a0> (a jcifs.smb.SmbTransport)
>         at jcifs.smb.SmbSession.logon(SmbSession.java:167)
>         at jcifs.smb.SmbSession.logon(SmbSession.java:162)
>
> "Thread-11":
>         at jcifs.smb.SmbTree.treeConnect(SmbTree.java:130)
>         - waiting to lock <0x1b90f2a0> (a jcifs.smb.SmbTransport)
>         - locked <0x1b9129a8> (a java.lang.Object)
>         at jcifs.smb.SmbSession.logon(SmbSession.java:169)
>         at jcifs.smb.SmbSession.logon(SmbSession.java:162)
>
>
> due to the dual locks in treeConnect
>
>     void treeConnect( ServerMessageBlock andx,
>                             ServerMessageBlock andxResponse ) throws
> SmbException {
>         String unc;
>         SmbTransport transport = session.transport();
>            synchronized(transport.setupDiscoLock) {
>                  synchronized(transport) {
>
>
> Other than radically changing the locking system I think reversing the order
> of these locks would really help the situation
> as the transport lock is being held routinely by most functions so by
> nesting these in the curent order (setupDiscoLock and transport)
> there is a race condition where any thread calling logoff will potentially
> end up calling treeconnect while already holding the transport lock..

This issue is well known. Unfortunately the fix is not trivial
(changing the order of locks or adding new ones cannot fix it) and I
don't have any time to work on it.

Mike

-- 
Michael B Allen
PHP Active Directory SPNEGO SSO
http://www.ioplex.com/