hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Roland Weber <http-as...@dubioso.net>
Subject Re: Connection Manager Garbage Collection
Date Mon, 23 Jul 2007 16:27:05 GMT
Hi Mike,

thanks for taking the time to look into this!

> REFERENCE_TO_CONNECTION_SOURCE will only have a reference to a
> ConnectionPool if a connection is checked out and hasn't been GCed (is
> this what you mean by allocated?).

Yes, that's what I meant by allocated.
An application got it, and keeps it.

>> 2. no connection is allocated. Then the TSCCM is GCed,
>>    we have a unit test for this. But how are the open
>>    connections closed in this scenario? I didn't see a
>>    finalizer that would close them.
> 
> Currently there is nothing that closes threads other than shutdown.

I was referring to the connections, but if shutdown isn't
called, then the pooled connections will just be left to
the GC and the built-in finalizer for open sockets, right?

> Not sure if we'd want to put this in a finalizer or not.  In general I
> have always avoided using them but perhaps this would be a good use
> case.

I have an idea how to handle this from the background thread.
But I'll put something into finalizers too, because I want to
make connection GC, and thereby the background thread, optional.

> I have also attached changes to the LostConnWorker that should
> alleviate the issue with the GC test case.

Unfortunately, the patch introduces threading (=GC) issues:

   while (this.workerThread == Thread.currentThread() &&
          connPoolRef.get() != null) {
       try {
            //@@@ (1)
            Reference ref = ((ConnectionPool) connPoolRef.get()
                             ).refQueue.remove();
            //@@@ (2)
            if (ref instanceof PoolEntryRef) {
              ((ConnectionPool) connPoolRef.get()
               ).handleReference((PoolEntryRef) ref);
            }
       } catch (InterruptedException e) {
         LOG.debug("LostConnWorker interrupted", e);
       }
   }

If GC runs at one of the positions marked @@@, then the
following dereferencing of the weak reference may trigger
a NullPointerException. The operation between (1) and (2)
is actually blocking until the GC puts something into the
reference queue, so that is the most likely point of failure.
One might argue that the NPX is thrown only when the pool
is GCed and the thread should die anyway, but I feel that
would be a bit too crude.

> remove all of the code related to the
> ReferenceQueueThread.  After that is done the only static code left is
> related to ALL_CONNECTION_MANAGERS.  Is the plan to keep or remove
> this code?

It needs to go. As long as we have shared, static maps in the
code, it's not safe for use in shared environments such as
OSGi (Felix, Eclipse) or servlet containers (when put in a
parent classloader).

So here is my dubious stratagem...

1. move TSCCM into a separate package
2. factor out some of the inner classes
  a) replace the nesting with weak or hard references
  b) introduce interfaces where needed
3. move static maps into a separate class which
   is marked for extinction
4. document the relations between the classes
   in a package.html
5. perform remaining cleanup, exterminate 3.
6. see what can be generalized

(not sure about the order of 4 and 5)

cheers,
  Roland



---------------------------------------------------------------------
To unsubscribe, e-mail: httpcomponents-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: httpcomponents-dev-help@jakarta.apache.org


Mime
View raw message