On 9/27/14 10:16 AM, Jacopo Cappellato wrote: > On Sep 27, 2014, at 6:46 PM, Gary Gregory <garydgreg...@gmail.com> wrote: > >> It is not enough to set them volatile because the class has some invariants >> that include more than one field. > For example, under unlucky thread concurrency the method getIdleTimeMillis() > could return a negative value.
Thanks for jumping in! See last response re DefaultPooledObject#getIdleTimeMillis. One thing to keep in mind when digging in to this code is that PooledObjectState is used internally by the pool implementations to effectively gate access to PooledObject instance data. Clients do not in general have direct access to these objects. While monitor contention would not be a practical issue, the overhead of adding sync locking to make DefaultPooledObject strictly threadsafe might have performance impacts. I would personally favor adding a disclaimer to the javadoc rather than adding more sync to this object. Alternatively, we could add a synchronized version of the class that factories could use, or just point out that if your factories require (for reasons not obvious to me) more sync protection on these objects you can subclass. The PooledObject interface was introduced so that factories could do exactly this kind of thing. If on the other hand you can identify thread-safety issues that could impact GOP / GKOP usage, we certainly want to address those. The issue above is an example. If my analysis is incorrect and you can engineer a test case showing incorrect stats being reported by the pool, then please do open a ticket. Thanks again for looking at the code. Phil > > Jacopo > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
