Thanks Martin, Mike, Chris and Daniel,
This has been pushed.
Regards, Peter
On 01/21/2015 05:44 PM, Martin Buchholz wrote:
Looks good to me!
On Wed, Jan 21, 2015 at 5:04 AM, Peter Levart <peter.lev...@gmail.com
<mailto:peter.lev...@gmail.com>> wrote:
Hi Martin and others,
I have also modified the test per Martin's suggestion to factor
out the serialClone() method. Here's the latest webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.03/
<http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/Hashtable.8068427/webrev.03/>
Is this ok now?
Regards, Peter
On 01/09/2015 11:16 AM, Peter Levart wrote:
On 01/05/2015 05:52 PM, Mike Duigou wrote:
Well spotted Peter. The change looks good though I wonder
if it should be:
int length = (int)((elements + elements / 20) /
loadFactor) + 3;
FYI, regarding Daniel's suggestion: When similar invariant
checks were added to the HashMap deserialization method we
found code which relied upon the illegal values. In some
cases the serialized HashMaps were created by alternative
serialization implementations which included illegal, but
until the checks were added, "harmless" values.
The invariant checks should still be added though. It
might even be worth adding checks that the other
deserialized values are in valid ranges.
Mike
Hi Mike and others,
Yes, your suggested length computation is more in "spirit"
with the comment above that states: "Compute new length with a
bit of room 5% to grow...", since it takes loadFactor into
account for that additional 5% too. I changed it to your
suggested expression.
Here's new webrev:
http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.02/
<http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/Hashtable.8068427/webrev.02/>
In addition to valid loadFactor, # of elements is checked to
be non-negative. The original length is just "repaired" if it
appears to be less than the enforced auto-growth invariant of
Hashtable.
I also expanded the test to exercise more combinations of # of
elements and loadFactor. Here's what gets printed with current
Hashtable implementation:
ser. deser.
size load lentgh length valid range ok?
------- ----- ------- ------- ----------------- ------
10 0.15 127 4 67... 134 NOT-OK
10 0.50 31 8 21... 42 NOT-OK
10 0.75 15 10 14... 28 NOT-OK
10 1.00 15 13 11... 22 OK
10 2.50 7 7 5... 10 OK
50 0.15 511 12 334... 668 NOT-OK
50 0.50 127 30 101... 202 NOT-OK
50 0.75 127 42 67... 134 NOT-OK
50 1.00 63 55 51... 102 OK
50 2.50 31 31 21... 42 OK
500 0.15 4095 103 3334... 6668 NOT-OK
500 0.50 1023 278 1001... 2002 NOT-OK
500 0.75 1023 403 667... 1334 NOT-OK
500 1.00 511 511 501... 1002 OK
500 2.50 255 255 201... 402 OK
5000 0.15 65535 1003 33334... 66668 NOT-OK
5000 0.50 16383 2753 10001... 20002 NOT-OK
5000 0.75 8191 4003 6667... 13334 NOT-OK
5000 1.00 8191 5253 5001... 10002 OK
5000 2.50 2047 2047 2001... 4002 OK
With patched Hashtable, the test passes:
ser. deser.
size load lentgh length valid range ok?
------- ----- ------- ------- ----------------- ------
10 0.15 127 69 67... 134 OK
10 0.50 31 23 21... 42 OK
10 0.75 15 15 14... 28 OK
10 1.00 15 13 11... 22 OK
10 2.50 7 7 5... 10 OK
50 0.15 511 349 334... 668 OK
50 0.50 127 107 101... 202 OK
50 0.75 127 71 67... 134 OK
50 1.00 63 55 51... 102 OK
50 2.50 31 23 21... 42 OK
500 0.15 4095 3501 3334... 6668 OK
500 0.50 1023 1023 1001... 2002 OK
500 0.75 1023 703 667... 1334 OK
500 1.00 511 511 501... 1002 OK
500 2.50 255 213 201... 402 OK
5000 0.15 65535 35003 33334... 66668 OK
5000 0.50 16383 10503 10001... 20002 OK
5000 0.75 8191 7003 6667... 13334 OK
5000 1.00 8191 5253 5001... 10002 OK
5000 2.50 2047 2047 2001... 4002 OK
Regards, Peter
On 2015-01-05 07:48,
core-libs-dev-requ...@openjdk.java.net
<mailto:core-libs-dev-requ...@openjdk.java.net> wrote:
Today's Topics:
2. Re: RFR: JDK-8068427 Hashtable deserialization
reconstitutes
table with wrong capacity (Daniel Fuchs)
Message: 2
Date: Mon, 05 Jan 2015 15:52:55 +0100
From: Daniel Fuchs <daniel.fu...@oracle.com
<mailto:daniel.fu...@oracle.com>>
To: Peter Levart <peter.lev...@gmail.com
<mailto:peter.lev...@gmail.com>>, core-libs-dev
<core-libs-dev@openjdk.java.net
<mailto:core-libs-dev@openjdk.java.net>>
Subject: Re: RFR: JDK-8068427 Hashtable
deserialization reconstitutes
table with wrong capacity
Message-ID: <54aaa547.8070...@oracle.com
<mailto:54aaa547.8070...@oracle.com>>
Content-Type: text/plain; charset=utf-8; format=flowed
On 04/01/15 18:58, Peter Levart wrote:
Hi,
While investigating the following issue:
https://bugs.openjdk.java.net/browse/JDK-8029891
I noticed there's a bug in deserialization code of
java.util.Hashtable
(from day one probably):
https://bugs.openjdk.java.net/browse/JDK-8068427
The fix is a trivial one-character replacement:
'*' -> '/', but I also
corrected some untruthful comments in the
neighbourhood (which might
have been true from day one, but are not any more):
http://cr.openjdk.java.net/~plevart/jdk9-dev/Hashtable.8068427/webrev.01/
<http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/Hashtable.8068427/webrev.01/>
Hi Peter,
I wonder whether there should be a guard against
loadFactor being
zero/negative/NaN after line 1173, like in the
constructor e.g. as
in lines
188 if (loadFactor <= 0 ||
Float.isNaN(loadFactor))
189 throw new
IllegalArgumentException("Illegal Load:
"+loadFactor);
if only to avoid division by zero.
best regards,
-- daniel
Regards, Peter
