[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-runtime-dev
Subject: Re: Request for review: 7197627: There are issues with shared data on windows
From: harold seigel <harold.seigel () oracle ! com>
Date: 2013-01-31 15:31:38
Message-ID: 510A8E5A.1040507 () oracle ! com
[Download RAW message or body]
Hi,
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 http://cr.openjdk.java.net/~hseigel/bug_7197672/
>>> <http://cr.openjdk.java.net/%7Ehseigel/bug_7197672/>
>>>
>>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=7197672
>>>
>>> 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
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi,<br>
<br>
Thank you for your comments. I plan to change the comment in
question to the following. Please let me know if that sounds
better:<br>
<blockquote> // Use remove() to delete the existing file because,
on Unix, this will<br>
// allow processes that have it open continued access to the
file.<br>
</blockquote>
I tried Ioi's experiment. On Windows, it worked as Dan expected.
On Linux, it worked as Ioi expected.<br>
<br>
Thanks, Harold<br>
<br>
On 1/30/2013 11:55 AM, Ioi Lam wrote:
<blockquote cite="mid:5109508F.8070702@oracle.com" type="cite">On
01/30/2013 06:20 AM, Daniel D. Daugherty wrote:
<br>
<blockquote type="cite">On 1/30/13 6:39 AM, harold seigel wrote:
<br>
<blockquote type="cite">Hi,
<br>
<br>
Please review the following change to fix bug 7197627.
<br>
<br>
Summary:
<br>
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.
<br>
<br>
This was tested on Windows 7 and Windows XP and sanity tested
on Linux.
<br>
<br>
Open webrev at
<a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~hseigel/bug_7197672/">http://cr.openjdk.java.net/~hseigel/bug_7197672/</a>
<a class="moz-txt-link-rfc2396E" \
href="http://cr.openjdk.java.net/%7Ehseigel/bug_7197672/"><http://cr.openjdk.java.net/%7Ehseigel/bug_7197672/></a>
<br>
<br>
Bug link at <a class="moz-txt-link-freetext" \
href="http://bugs.sun.com/view_bug.do?bug_id=7197672">http://bugs.sun.com/view_bug.do?bug_id=7197672</a>
<br>
<br>
Thanks! Harold
<br>
</blockquote>
<br>
Thumbs up on the safer change.
<br>
<br>
This comment:
<br>
<br>
217 // Remove the existing file in case another process has it
open.
<br>
<br>
bothers me. On Windows, if another process has the classes.jsa
file
<br>
open, then you won't be able to remove it (or move it or...) so
the
<br>
comment is misleading. Or I'm wrong about Windows which could
easily
<br>
be true...
<br>
<br>
Dan
<br>
<br>
</blockquote>
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.
<br>
<br>
Maybe we need to do an #ifdef around this comment?
<br>
<br>
- Ioi
<br>
</blockquote>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic