On 03/02/2009, Oleg Kalnichevski <[email protected]> wrote: > 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. >
That's probably true for PCs and smaller systems, however larger servers often have several caches, some of which may be large and long lasting. For example, I used to work on Alpha systems, and writes to the main memory from one CPU could be seen in a different order on another CPU. [I understand this was done for performance reasons.] So a thread on one CPU only sees the correct data if both CPUs issue the appropriate memory barrier instructions in the correct order. > > 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. > Final fields have additional properties - according to Java Concurrency In Practice: "... any variables that can be reached through a final field of a properly constructed object ... such as .. the contents of a HashMap referenced by a final field are guaranteed to be visible to other threads ... so long as the objects are only reachable through final fields of the object under construction." So I think we can gain something. I agree it's quite unlikely that the data won't be visible, as other activities will probably cause memory flushes, but it seems risky to me to depend on that. > 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] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
