On 12.02.2013 20:41, Florent Guillaume wrote:
> On Tue, Feb 12, 2013 at 7:36 PM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> I see. I think there's yet another potential explanation: even if glassfish
>> is careful to invoke the end() only after start() has fully finished in the
>> other thread, the java memory model does not guarantee that the effects of
>> the end() in one thread are visible to the other thread doing the start().
>> The update of the PGXAConnection's state-variable might be sitting in the
>> CPU cache of the CPU running the first thread, not yet visible to the second
>> thread.
>
> That's the whole point of volatile since Java 5, it enforces a barrier
> and "happens-before" semantics.
>
>> However, I'm not sure if glassfish could guarantee that the start()
>> has really finished before end() is called, without using some operation
>> that would also force the CPU cache to be flushed, making the effects
>> visible.
>>
>> In any case, it seems like we should add "synchronized" to all the public
>> methods in PGXAConnection. The performance penalty should be minimal, and it
>> would fix any race conditions of that sort.
>
> For Java 5+ this is really overkill.
Well, volatile will only fix the issue if it's indeed "just" because of
weak memory ordering issues. However, if the cause is a genuine race
condition where glassfish calls end() in one thread before the start()
in another thread has ran to completion, as Bryan suspected, then
volatile is not enough. Besides, it would be good for PGXAConnection to
be thread-safe in general, like the rest of the JDBC driver.
- Heikki