You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by Roland Weber <ht...@dubioso.net> on 2007/01/21 15:01:24 UTC

[HttpConn] connection manager - volatile

Hi all,

the MTHCM has two shutdown attributes, one for the connection
manager and one for the ReferenceQueueThread. If I am not
mistaken, these attributes should be declared 'volatile'
since they are used from different threads?
The ReferenceQueueThread wakes up every second just to check
the shutdown status. If the attribute is declared volatile
and RQT.shutdown() modified to interrupt the thread, would
that work without polling?

MTHCM.shutdownAll() might be a bit unsafe since it iterates
over a WeakHashMap and does not handle ConcurrentModifcation-
Exception:
http://java.sun.com/j2se/1.4.2/docs/api/java/util/WeakHashMap.html

MTHCM.shutdownAll() synchronizes on the map for all the weak
references. I assume this to be an optimization rather than
a requirement to prevent deadlocks. Is that correct?

cheers,
  Roland

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


Re: [HttpConn] connection manager - volatile

Posted by Roland Weber <ht...@dubioso.net>.
Hi Mike,

> Assuming that keySet().toArray() is implemented to work around the CMX
> I would agree.

I expect it to be. CMX is only documented for iterators, and they should
not rely on an iterator for toArray(). I expect them to call toArray() on
the underlying map holding WeakReference objects, followed by a traversal
of the array to dereference into the result array. In the worst case, the
result array has some null entries.

cheers,
  Roland


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


Re: [HttpConn] connection manager - volatile

Posted by Michael Becke <mb...@gmail.com>.
> > Seems like catching the exception and trying again should be enough.
>
> That was my first idea too. But now I think toArray() is the more
> elegant solution. I'll come up with a patch for 3.1 this week-end
> at latest.

Assuming that keySet().toArray() is implemented to work around the CMX
I would agree.

Mike

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


Re: [HttpConn] connection manager - volatile

Posted by Roland Weber <ht...@dubioso.net>.
Hi Mike,

>> So they can not _guarantee_ that a CMX is thrown if the GC drops
>> something from the map during the iteration, but they'll try to.
> 
> Seems like catching the exception and trying again should be enough.

That was my first idea too. But now I think toArray() is the more
elegant solution. I'll come up with a patch for 3.1 this week-end
at latest.

cheers,
  Roland


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


Re: [HttpConn] connection manager - volatile

Posted by Michael Becke <mb...@gmail.com>.
> The garbage collector. The class comment for WeakHashMap is full
> of warnings like this one:
>
>    Because the garbage collector may discard keys at any time,
>    a WeakHashMap may behave as though an unknown thread is silently
>    removing entries. In particular, even if you synchronize on a
>    WeakHashMap instance and invoke none of its mutator methods,
>    it is possible...
>
> In 1.4, they added some sections:
>
>    Note that the fail-fast behavior of an iterator cannot be
>    guaranteed as it is, generally speaking, impossible to make
>    any hard guarantees in the presence of unsynchronized
>    concurrent modification.
>
> So they can not _guarantee_ that a CMX is thrown if the GC drops
> something from the map during the iteration, but they'll try to.

Yes, didn't read this carefully enough.  They seem to be saying that
they cannot guarantee most default behavior of this map due to GC.
Seems like catching the exception and trying again should be enough.

> Yes, I've already been thinking about moving the reference tracking
> to an extra class. Some of my question had exactly that background.
> But there are more important things to be taken care of first, so
> I'll let this settle for a while. Except for the volatile/non-polling
> stuff. Maybe I'll come up with a patch for 3.1.

Yep, separating is not a high priority.

Mike

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


Re: [HttpConn] connection manager - volatile

Posted by Roland Weber <ht...@dubioso.net>.
Hi Mike,

>> MTHCM.shutdownAll() might be a bit unsafe since it iterates
>> over a WeakHashMap and does not handle ConcurrentModifcation-
>> Exception:
>> http://java.sun.com/j2se/1.4.2/docs/api/java/util/WeakHashMap.html
> 
> Not sure.  In what scenarios would this occur do you think?  What
> other thread would be modifying the map?

The garbage collector. The class comment for WeakHashMap is full
of warnings like this one:

   Because the garbage collector may discard keys at any time,
   a WeakHashMap may behave as though an unknown thread is silently
   removing entries. In particular, even if you synchronize on a
   WeakHashMap instance and invoke none of its mutator methods,
   it is possible...

In 1.4, they added some sections:

   Note that the fail-fast behavior of an iterator cannot be
   guaranteed as it is, generally speaking, impossible to make
   any hard guarantees in the presence of unsynchronized
   concurrent modification.

So they can not _guarantee_ that a CMX is thrown if the GC drops
something from the map during the iteration, but they'll try to.

> When we get around to refactoring/rewriting the MTHCM ideally it would
> be best if the weak references, queues, etc could be separated from
> the connection pool.  This code ads a lot of complexity, requires a
> thread, and when connections and managers are used correctly should
> not even be necessary.  At the moment I have no idea how exactly this
> could be done, but I thought it should be considered.

Yes, I've already been thinking about moving the reference tracking
to an extra class. Some of my question had exactly that background.
But there are more important things to be taken care of first, so
I'll let this settle for a while. Except for the volatile/non-polling
stuff. Maybe I'll come up with a patch for 3.1.

cheers,
  Roland


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


Re: [HttpConn] connection manager - volatile

Posted by Michael Becke <mb...@gmail.com>.
> the MTHCM has two shutdown attributes, one for the connection
> manager and one for the ReferenceQueueThread. If I am not
> mistaken, these attributes should be declared 'volatile'
> since they are used from different threads?

Agreed.  Should be volatile.

> The ReferenceQueueThread wakes up every second just to check
> the shutdown status. If the attribute is declared volatile
> and RQT.shutdown() modified to interrupt the thread, would
> that work without polling?

Yes, I think you're right.  Removing the polling is the way to go.

> MTHCM.shutdownAll() might be a bit unsafe since it iterates
> over a WeakHashMap and does not handle ConcurrentModifcation-
> Exception:
> http://java.sun.com/j2se/1.4.2/docs/api/java/util/WeakHashMap.html

Not sure.  In what scenarios would this occur do you think?  What
other thread would be modifying the map?

> MTHCM.shutdownAll() synchronizes on the map for all the weak
> references. I assume this to be an optimization rather than
> a requirement to prevent deadlocks. Is that correct?

It prevents deadlocks and makes sure that no connections are allocated
while things are being shutdown.  It also allows for the
ReferenceQueueThread to be restarted if/when new MTHCH instances are
created.

When we get around to refactoring/rewriting the MTHCM ideally it would
be best if the weak references, queues, etc could be separated from
the connection pool.  This code ads a lot of complexity, requires a
thread, and when connections and managers are used correctly should
not even be necessary.  At the moment I have no idea how exactly this
could be done, but I thought it should be considered.

Mike

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