Request for review 6924920: Class Data Sharing limit on the java version string can create failures: JVM Ident field too long

[David Holmes]

On 10/11/2012 6:08 AM, harold seigel wrote:
> Hi Peter,
>
> We plan to use something similar to what you suggested. If the version
> string is < JVM_IDENT_MAX then it will not get truncated. Any version
> string >= JVM_IDENT_MAX will get truncated and the last N bytes of the
> string will be filled in with hex digits from a hash. Code that then
> reads this version string can tell whether or not the string has been
> hashed based on its length.

This seems excessive. You could still get a hash collision and the 
likelihood of that depends on the hashing algorithm and the likely 
differences in such long version strings (which you don't know). Plus 
you can always build two quite different VMs that have the same version 
string!

As I understand it this is meant to be a basic sanity check not a 
foolproof identification scheme - if you want the latter then generate a 
GUID on every build and use that.

David
-----

> Thanks for the suggestion!
> Harold
>
> On 11/9/2012 11:46 AM, Peter Levart wrote:
>> On 11/09/2012 05:40 AM, David Holmes wrote:
>>> On 9/11/2012 5:48 AM, Coleen Phillimore wrote:
>>>>
>>>> Harold,
>>>> I looked at this once and thought it was okay, but now I don't think it
>>>> is. I think you want fail_stop() but just not to print the string that
>>>> isn't initialized.
>>>> fail_continue() is used when reading the archive. It allows the vm to
>>>> continue without using the shared archive. But when you're dumping it,
>>>> if the string is truncated, it won't create a usable shared archive
>>>> anyway. The fail_continue() closes the CDS archive file.
>>>>
>>>> If you want to have a truncated string work, you have issue the
>>>> fail_continue() message as a warning only. Then change the code for
>>>> Xshare:on that validates that the versions match to truncate the
>>>> current_version() too.
>>>>
>>>> I suggest leaving it as fail_stop() but fixing the message to not print
>>>> uninitialized data.
>>>
>>> I agree that fail_continue is the wrong choice. But it seems to me
>>> that when the ident is truncated all that is needed is a warning
>>> printed. AFAICS the archive will still be usable with a truncated
>>> ident and it will match when read back in during validate().
>>
>> Hi,
>>
>> But wouldn't also match when read back with a version string that
>> differs from the writer's version string only in chars past 255? Are
>> those chars significant?
>>
>> What about truncating the version string to 255-32 = 223 bytes and
>> appending (only if truncated) a 32 hex digits of a MD5 of the whole
>> version string? No warnings.
>>
>> Regards, Peter
>>
>>>
>>> David
>>> ------
>>>
>>>>
>>>> Coleen
>>>>
>>>> On 11/8/2012 1:23 PM, harold seigel wrote:
>>>>> Please review the following change.
>>>>>
>>>>> Summary: The problem was fixed by truncating the JVM ident to
>>>>> JVM_IDENT_MAX and calling method fail_continue() instead of
>>>>> fail_stop().
>>>>>
>>>>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_6924920/
>>>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_6924920/>
>>>>>
>>>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=6924920
>>>>>
>>>>> The changes were tested with JPRT, JCK, and by using a version string
>>>>> that exceeded 256 characters.
>>>>>
>>>>> Thanks, Harold
>>