RFR(XS) 8190809 JVM crashes while generating appcds for classpath with empty directory entry

Leonid Mesnik leonid.mesnik at oracle.com
Fri Dec 1 19:44:56 UTC 2017


Thank you for update. Looks good.

Leonid

> On Dec 1, 2017, at 11:42 AM, Ioi Lam <ioi.lam at oracle.com> wrote:
> 
> Hi Leonid,
> 
> Thanks for the review. I've changed that line to
> 
>         File dir = new File(System.getProperty("user.dir"));
> (which is similar to what TestCommon.java does with other temp files).
> 
> I'll rerun the tests now.
> 
> Thanks
> 
> - Ioi
> 
> On 12/1/17 11:32 AM, Leonid Mesnik wrote:
>> Fix looks good.
>> The only one question. Why don’t just always use current dir in 39?
>> 39         File dir = new File(System.getProperty("test.classes", "."));
>> I think that jtreg always run test in new scratch directory however “test.classes” might be re-used between runs and people could have fail to create new directory if they re-run test locally.
>> 
>> Leonid
>> 
>>> On Dec 1, 2017, at 10:53 AM, Ioi Lam <ioi.lam at oracle.com <mailto:ioi.lam at oracle.com>> wrote:
>>> 
>>> Hi
>>> 
>>> Please review this very small change. I hope to invoke the trivial rule so that I can push before today's 8PM PDT deadline for JDK 10 integration.
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8190809 <https://bugs.openjdk.java.net/browse/JDK-8190809>
>>> http://cr.openjdk.java.net/~iklam/jdk10/8190809-cds-crash-empty-cp.v01/ <http://cr.openjdk.java.net/~iklam/jdk10/8190809-cds-crash-empty-cp.v01/>
>>> 
>>> The crash happened because CDS was trying to use a classpath as a JAR file, before checking it's actually a JAR file.
>>> 
>>> I added test case for the use of directories in -cp during CDS dumping, both for empty and non-empty cases.
>>> 
>>> I will test with tier1/tier2 testing before pushing.
>>> 
>>> Thanks
>>> - Ioi
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> PS: the check for non-emptiness is done in here:
>>> 
>>> void ClassLoader::check_shared_classpath(const char *path) {
>>>   if (strcmp(path, "") == 0) {
>>>     exit_with_path_failure("Cannot have empty path in archived classpaths", NULL);
>>>   }
>>> 
>>>   struct stat st;
>>>   if (os::stat(path, &st) == 0) {
>>>     if ((st.st_mode & S_IFMT) != S_IFREG) { // is not a regular file
>>>       if (!os::dir_is_empty(path)) {
>>>         tty->print_cr("Error: non-empty directory '%s'", path);
>>>         exit_with_path_failure("CDS allows only empty directories in archived classpaths", NULL);
>>>       }
>>>     }
>>>   }
>>> }
>>> 
>>> 
>> 
> 



More information about the hotspot-runtime-dev mailing list