Sunbug 6934356: Vector.writeObject() synchronization risks serialization deadlock

[Neil Richards]

On 16 December 2010 13:01, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> I've looked at the patch and don't see anything obviously wrong. I think
> Mike might is looking at it too. It clearly comes with a performance cost of
> course.

I've tried to write the fix so that the cost is minimized.

> One small comment is that the fields are partly populated in the
> synchronized block and partly outside. It might be cleaner to take a copy of
> the capacity increment and element count in the synchronized block and then
> set all the fields together outside.

I guess I did as little data copying as was necessary to make the fix,
to minimize its performance cost.

> Another minor comment is the "final"s
> don't seem to be needed.

Yeah, I admit my tendency to declare variables as 'final' (where they
can be) is a little idiosyncratic.

I find that the use of 'final' helps to make (reading) the code
clearer - once you've seen a field/variable is final, you're happy
it's not going to change.

Also, I suspect declaring things as 'final' might also give extra
clues to JITs on how to optimally compile the code.

> On the test case, it needs the GPL header. There are templates in the
> repository and you'll also see tests contributed by RedHat and Google if
> that helps.

Ah, sorry. I'll work on getting the right words in there, and will
attach a modified changeset as soon as I have it sorted.

> Another thing to mention is that hard-coded timeouts are
> problematic. In this case a 1s timeout might not be sufficient when on a
> machine that is running many tests concurrently. One suggestion is to just
> leave the threads deadlock and the test will timeout and fail if the bug
> exists.

Yes, I worried over the timeout aspect of the testcase, too.

I had guessed that a testcase that hung forever was worse behaved than
one which reported failure after a certain timeout period - at least,
when the hang is the expected failure mode for the test.

But I see that it can be sensitive to the load on the machine it's being run on.

So I'm happy to modify the test if the jtreg test harness deals with
cleaning up hanging tests, reporting them as failures.

Cheers,
Neil

--
Unless stated above:
Work email: neil_richards at uk.ibm.com
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU