advice & review requested for 6896297, race condition in rmid causing JCK failure
stuart.marks at oracle.com
Tue Apr 12 22:08:43 UTC 2011
On 4/11/11 7:46 AM, Alan Bateman wrote:
> Stuart Marks wrote:
>> Hi Core-Libs developers,
>> I'd like to solicit some advice and discussion about this bug and a potential
>> fix I'm cooking for it. Here is the bug report; it contains details about the
>> problem and my analysis of it:
>> and here's a webrev of the fix I'm working on:
> I haven't worked on RMI and so I don't have a good insight into the
> implementation. I think (and Peter Jones can correct me) that changing the
> serialized form would just inhibit recovery of rmid from an existing log file.
> As David said, you could switch to CHM and implement readObject/writeObject to
> serialize/deserialize in the current form. That said, I looked your webrev and
> the changes look reasonable to me. I don't see anything obviously wrong with
> moving the unregisterGroup out of the synchronized block.
I didn't see anything obviously wrong either... my hope (or fear) was that
somebody would point out something non-obviously wrong. :-)
In a face-to-face conversation, Joe Darcy pointed out to me that serialization
compatibility is over the types of the fields declared in the class, not the
actual serial data in the object stream. The groupTable and idTable fields are
both Maps, so we could change the implementation from HashMap to
ConcurrentHashMap and it would be compatible with any implementation that has
CHM, i.e. 1.5 or later. (It would probably be wise to have readObject
reconstitute the tables as CHM in case they encountered HM in the serialized form.)
The alternatives seem to be as follows:
1. Convert groupTable and idTable to ConcurrentHashMap and remove external
synchronization, and otherwise make minimal changes to serialization code. This
puts a CHM into the object stream, which should be compatible with anything
using JDK 1.5 or later as noted above.
2. Convert groupTable and idTable to ConcurrentHashMap and remove external
synchronization, and provide readObject/writeObject implementations to place
ordinary HashMaps (as opposed to CHMs) in the object stream. This serialized
form is more compatible, but it requires more serialization code to be
developed, tested, and maintained.
3. Adjust the locking strategy to prevent ConcurrentModificationException but
otherwise make no changes to serialization or data structures. This is what
I've posted in the webrev.
The key issue seems to be whether we want to preserve compatibility of the
serialized form. If we care about compatibility of the serialized form, #3 is
probably the safest. If somebody comes along and tells us that we don't have to
worry about serial compatibility of this stuff at all, then #1 is probably the
best since it simplifies the code a lot.
In the absence of any further information, I'm inclined to go with #3, since
I've already developed and tested it and it's basically ready to go.
Any further thoughts, anybody?
More information about the core-libs-dev