Request for review: 7197627: There are issues with shared data on windows

harold seigel harold.seigel at
Thu Jan 31 07:31:38 PST 2013


Thank you for your comments.  I plan to change the comment in question 
to the following.  Please let me know if that sounds better:

       // Use remove() to delete the existing file because, on Unix,
    this will
       // allow processes that have it open continued access to the file.

I tried Ioi's experiment.  On Windows, it worked as Dan expected.  On 
Linux, it worked as Ioi expected.

Thanks, Harold

On 1/30/2013 11:55 AM, Ioi Lam wrote:
> On 01/30/2013 06:20 AM, Daniel D. Daugherty wrote:
>> On 1/30/13 6:39 AM, harold seigel wrote:
>>> Hi,
>>> Please review the following change to fix bug 7197627.
>>> Summary:
>>> The original fix for this bug created the CDS classes.jsa file with 
>>> read/write protection.  This change creates the file with read-only 
>>> protection and only changes the file/s protection to read/write when 
>>> deleting the file.  This is a safer implementation.
>>> This was tested on Windows 7 and Windows XP and sanity tested on Linux.
>>> Open webrev at 
>>> <>
>>> Bug link at
>>> Thanks!  Harold
>> Thumbs up on the safer change.
>> This comment:
>> 217   // Remove the existing file in case another process has it open.
>> bothers me. On Windows, if another process has the classes.jsa file
>> open, then you won't be able to remove it (or move it or...) so the
>> comment is misleading. Or I'm wrong about Windows which could easily
>> be true...
>> Dan
> This comment still applies to *nix. The remove() call will remove the 
> file from the file system, but the other process holding a file ID to 
> it will still be able to access the old contents. A good test case 
> would be one app running with -Xshare:on while you run -Xshare:dump 
> from a different terminal.
> Maybe we need to do an #ifdef around this comment?
> - Ioi
-------------- next part --------------
An HTML attachment was scrubbed...

More information about the hotspot-runtime-dev mailing list