On Tue, 2009-02-03 at 14:29 +0000, sebb wrote: > On 03/02/2009, Oleg Kalnichevski <[email protected]> wrote: > > On Mon, 2009-02-02 at 22:44 +0000, sebb wrote: > > > Protocol Interceptors are supposed to be thread-safe. > > > > > > However, BasicHttpProcessor is not thread-safe as changes to the Lists > > > are not synchronized. > > > > > > The rest of the Interceptors have no fields so are immutable. They > > > operate on non-thread-safe classes, however I assume that such classes > > > are confined to a single thread. > > > > > > == > > > > > > SessionRequestImpl is supposed to be thread-safe, however there are > > > several mutable fields that are not synch or volatile. > > > > > > == > > > > > > Are HttpParams supposed to be shared between threads? The Javadoc say > > > that they are intended to be immutable once initialised; however this > > > does not guarantee that the values will be visible to all threads. > > > > > > BasicHttpParams could be made less thread unsafe by making the map > > > final, and clear()ing rather than null-ing it. > > > > > > Likewise, if there was a copy constructor, if could save the map > > > variable after creating it. > > > > > > > > > (1) Both BasicHttpParams and BasicHttpProtocols can be shared between > > threads but they are expected to be used in the 'write once / read many' > > mode. Therefore, access to their internal structures is not synchronized > > for performance reason. These classes are not fully thread-safe if > > modified at runtime and any statement to the contrary is a mistake in > > documentation. > > I think we are talking about two different kinds of thread-safety here > - mutual exclusion and data visibility. > > I agree that if the classes are used in write once mode, then the > question of mutual exclusion does not arise. > > However, I'm referring to data visibility, which is about ensuring > that all threads see the same values. > > If a thread writes to a field, the updated contents of that field are > not necessarily visible to another thread - ever - as the JVM is free > to cache values locally. >
Sebastian, In technical terms variable caching means pretty much one thing only: the JVM may choose to keep a value of a non-volatile variable in a CPU register for performance optimization sake. Naturally other threads would not see that value when the contexts are switched. However, this can be a problem _only_ if the value stored in a CPU register _mutates_, which _should_ never occur as long as BasicHttpParams and BasicHttpProcessor are used as specified given the fact that register optimizations are very, very short-lived. > In order for fields to be shared between threads, a field must be > correctly published, e.g. by making it final. > > > I suggest we improve the documentation of BasicHttpParams and > > BasicHttpProtocols with regards to their intended usage, but will not > > make them fully thread-safe. Making those classes less prone to > > synchronization issues would also be welcome. > > > > Would that be okay for you? > > If the classes can be changed to use final for all shared variables, > then that should be enough to ensure that the data is published OK. > Internally BasicHttpParams is backed by a HashMap instance, BasicHttpProcessor is backed by two ArrayList instance. There is no problem making references to those instances final. However, this will not guarantee in any way that the internal variables of those instances are also final. Basically we gain nothing. Oleg > > (2) I'll review SessionRequestImpl > > > > Oleg > > > > > S/// > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [email protected] > > > For additional commands, e-mail: [email protected] > > > > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
