I checked the Connection class further and below are my thoughts:
1. Connection.connect() calls for waitForThread(null) -> Waiting for
ReaderThread to shutdown incase there is already one. But as I said
before, still the new ReaderThread is launched though the older one is
running.
2. As I debugged through waitForReader(), I found that it reaches the
condition (thread == deadReader and thread == null) and exits. This
does not go with the expectation of the method (that it will exit only
after the reader has completed)
My guess is the condition should be (reader == deadReader and thread
== null). I modified the logic based on that and now the deadlock
doesn't happen. But still, I'm not convinced that this is the fix and
it will not break anything.
I have pasted the changed code below. Could somebody review this and
pass on their comments (on top of Connection.java, 1.87)
Thanks,
Krishna R
>diff Connection.java.orig Connection.java.fix
363,367c363,368
< /*
< * The reader thread may start and immediately
terminate.
< * To prevent the waitForReader from waiting forever
< * for the dead to rise, we leave traces of the
deceased.
< * If the thread is already gone, we throw an
exception.
---
> /**
> * Logic:
> * 1. If shutdown request, wait until reader == deadReader
> * 2. If start request, wait until reader == thread (checked in outer while loop)
> * Or if reader == deadReader, in which case throw an exception
369,382c370,397
< if( thread == deadReader) {
< if (thread == null) /* then we wanted a shutdown
*/
< return;
< if( Debug.LDAP_DEBUG) {
< Debug.trace( Debug.messages, name +
< "reader already terminated, throw
exception");
< }
< IOException lex = deadReaderException;
< deadReaderException = null;
< deadReader = null;
< // Reader thread terminated
< throw new LDAPException(
< ExceptionMessages.CONNECTION_READER,
< LDAPException.CONNECT_ERROR, null, lex);
---
> if( thread == null) {
> /*
> * Since this is a reader thread shutdown request,
> * return since the reader thread is dead.
> */
> if (reader == deadReader) /* then we wanted a shutdown */
> return;
> }
> else {
> /*
> * The reader thread may start and immediately terminate.
> * To prevent the waitForReader from waiting forever
> * for the dead to rise, we leave traces of the deceased.
> * If the thread is already gone, we throw an exception.
> */
> if( thread == deadReader) {
> if( Debug.LDAP_DEBUG) {
> Debug.trace( Debug.messages, name +
> "reader already terminated, throw exception");
> }
> IOException lex = deadReaderException;
> deadReaderException = null;
> deadReader = null;
> // Reader thread terminated
> throw new LDAPException(
> ExceptionMessages.CONNECTION_READER,
> LDAPException.CONNECT_ERROR, null, lex);
> }
