Re: Hung thread in SmbTree.java

On Wed, Feb 18, 2015 at 2:52 PM, Rich Hammond <[email protected]> wr=
ote:
> I tried to search the archives and I do not see this issue.  If there is
> another place I should search, please let me know.
> I have a thread that has been hung for days at:
>
>    java.lang.Thread.State: WAITING (on object monitor)
>
>     at java.lang.Object.wait(Native Method)
>
>     - waiting on <0x000000074edf9e60> (a jcifs.smb.SmbTransport)
>
>     at java.lang.Object.wait(Object.java:503)
>
>     at jcifs.smb.SmbTree.treeConnect(SmbTree.java:143)
>
>     - locked <0x000000074edf9e60> (a jcifs.smb.SmbTransport)
>
>     at jcifs.smb.SmbFile.doConnect(SmbFile.java:911)
<snip>
>
>
> Looking at the sources for SmbTree I have a couple questions:
>
> In the treeConnect method why does the catch bother to call treeDisconnec=
t?
> The connectionState =3D 1 and treeDisconnect returns immediately in that =
case,
> so why bother calling at all?

Hi Rich,

No. It is possible for connectionState to change after set to 1
because something could call a method that calls wait() on the
transport which would unlock it. And in fact, session.send() calls
transport.send() which ultimately calls wait() on the transport.

> Should not the rule be that anyplace that one sets connectionState =3D 0;=
 you
> also call session.transport.notifyAll()?  Why doesn=E2=80=99t the catch b=
lock have a
> notifyAll()?

Yes. It looks like there is a flaw here. I have added this issue to
the TODO list.

But in this case I might try to just set connectionState =3D 0 in the
treeDisconnect where it returns and thus dodges the notifyAll and then
instead let it fall through to the existing notifyAll():

    void treeDisconnect( boolean inError ) {
synchronized (session.transport()) {

        if (connectionState !=3D 2) { // not-connected
            if (inError) {
                connectionState =3D 0;
            }
        } else {
            ...
        }

        session.transport.notifyAll();
}
    }

So the philosophy is that whenever connectionState changes to
anything, you always call notifyAll. There should not have been a
return in this block.

But this sort of code is non-trivial. I would have to play with it and
test it to make sure it's good so this is not the official fix.

Again, I put this on the TODO list for further investigation.

Thanks a lot for the feedback. Nice catch!

Mike

--
Java Active Directory Integration
http://www.ioplex.com/