RFR: JDK-8227021: VM fails if any sun.boot.library.path paths are longer than JVM_MAXPATHLEN

Daniel D. Daugherty daniel.daugherty at oracle.com
Tue Aug 13 16:57:50 UTC 2019 

Just realized that I forgot to include this link:

https://wiki.openjdk.java.net/display/Build/Submit+Repo

Sorry about that...

Dan


On 8/13/19 12:17 PM, Daniel D. Daugherty wrote:
> Let's see what Serguei (or David) says about testing needed.
> Looks like he reviewed this change (and your other one too)...
>
> I haven't reviewed either change... I just happened to notice
> that nothing was said in the RFR about testing...
>
> Dan
>
>
> On 8/13/19 12:14 PM, Adam Farley8 wrote:
>> Hi Daniel,
>>
>> As this change isn't a part of the class libraries, and this code 
>> path is exercised during startup, I neglected to run testing beyond 
>> the bug specific tests and variants theron.
>>
>> I'm happy to take recommendations for the tests I should run.
>>
>> I'm not familiar with jdk_submit. Google isn't helping, nor is a 
>> jdk/jdk search. Can you advise on how I can run it?
>>
>> Best Regards
>>
>> Adam Farley
>> IBM Runtimes
>>
>>
>> "Daniel D. Daugherty" <daniel.daugherty at oracle.com> wrote on 
>> 13/08/2019 17:05:20:
>>
>> > From: "Daniel D. Daugherty" <daniel.daugherty at oracle.com>
>> > To: Adam Farley8 <adam.farley at uk.ibm.com>
>> > Cc: David Holmes <david.holmes at oracle.com>, hotspot-dev at openjdk.java.net
>> > Date: 13/08/2019 17:05
>> > Subject: Re: RFR: JDK-8227021: VM fails if any sun.boot.library.path
>> > paths are longer than JVM_MAXPATHLEN
>> > 
>> > Hi Adam,
>> >
>> > I should have been more clear. The code review request should indicate
>> > what kind of testing has been done on the change. Could be something
>> > like:
>> >
>> > - Ran through jdk_submit with no failures.
>> > - Also ran the test from the bug report on Linux or Win* or XXX.
>> >
>> > So I see that you've run the bug specific test. Did you run any other
>> > testing like jdk_submit?
>> >
>> > Dan
>> >
>>
>> > On 8/13/19 12:00 PM, Adam Farley8 wrote:
>> > Hi Daniel, 
>> >
>> > The basic test case is described in the bug:
>> >
>> > java -Dsun.boot.library.path=$LONGPATH:$RIGHTPATH AnyClass
>> >
>> > (where LONGPATH has >5k characters in it)
>> >
>> > As this code path is executed during library loading at startup,
>> > this test case should be enough to demonstrate the fix.
>> >
>> > Does this answer your question?
>> >
>> > Best Regards
>> >
>> > Adam Farley
>> > IBM Runtimes
>> >
>> >
>> > "Daniel D. Daugherty" <daniel.daugherty at oracle.com> wrote on 13/08/
>> > 2019 16:51:56:
>> >
>> > > From: "Daniel D. Daugherty" <daniel.daugherty at oracle.com>
>> > > To: Adam Farley8 <adam.farley at uk.ibm.com>, David Holmes
>> > > <david.holmes at oracle.com>
>> > > Cc: hotspot-dev at openjdk.java.net
>> > > Date: 13/08/2019 16:52
>> > > Subject: Re: RFR: JDK-8227021: VM fails if any sun.boot.library.path
>> > > paths are longer than JVM_MAXPATHLEN
>> > >
>> > > I don't see any information describing how this change was tested.
>> > >
>> > > Dan
>> > >
>> > >
>> > > On 8/13/19 11:48 AM, Adam Farley8 wrote:
>> > > > Hi David,
>> > > >
>> > > > Since we have two positive reviews (yours and Serguei's), could you
>> > > > sponsor the change please?
>> > > >
>> > > > Best Regards
>> > > >
>> > > > Adam Farley
>> > > > IBM Runtimes
>> > > >
>> > > >
>> > > > David Holmes <david.holmes at oracle.com> wrote on 12/08/2019 
>> 23:32:14:
>> > > >
>> > > >> From: David Holmes <david.holmes at oracle.com>
>> > > >> To: Adam Farley8 <adam.farley at uk.ibm.com>
>> > > >> Cc: hotspot-dev at openjdk.java.net, "serguei.spitsyn at oracle.com"
>> > > >> <serguei.spitsyn at oracle.com>
>> > > >> Date: 12/08/2019 23:32
>> > > >> Subject: Re: RFR: JDK-8227021: VM fails if any 
>> sun.boot.library.path
>> > > >> paths are longer than JVM_MAXPATHLEN
>> > > >>
>> > > >> Looks fine.
>> > > >>
>> > > >> Thanks,
>> > > >> David
>> > > >>
>> > > >> On 13/08/2019 1:19 am, Adam Farley8 wrote:
>> > > >>> Hi David,
>> > > >>>
>> > > >>> Changes made as requested:
>> > > >>> https://urldefense.proofpoint.com/v2/url?
>> > > >> u=http-3A__cr.openjdk.java.net_-7Eafarley_8227021.4_webrev&d=DwID-
>> > > >> g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-
>> > > >>
>> > > >
>> > >
>> > 
>> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=mZM8IR4e_zmrfcsF3XJjoGEMrZb4WtEL7Y6Ugd6Naqg&s=sZ2UKqqUIq0El-
>> > > >> RsqYz6jmTh4Q2UghwdrQX6of8Lw0E&e=
>> > > >>> Best Regards
>> > > >>>
>> > > >>> Adam Farley
>> > > >>> IBM Runtimes
>> > > >>>
>> > > >>>
>> > > >>> David Holmes <david.holmes at oracle.com> wrote on 12/08/2019 
>> 04:55:36:
>> > > >>>
>> > > >>>> From: David Holmes <david.holmes at oracle.com>
>> > > >>>> To: Adam Farley8 <adam.farley at uk.ibm.com>,
>> > > >>>> "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
>> > > >>>> Cc: hotspot-dev at openjdk.java.net
>> > > >>>> Date: 12/08/2019 04:56
>> > > >>>> Subject: Re: RFR: JDK-8227021:  VM fails if any 
>> sun.boot.library.path
>> > > >>>> paths are longer than JVM_MAXPATHLEN
>> > > >>>>
>> > > >>>> Hi Adam,
>> > > >>>>
>> > > >>>> On 10/08/2019 2:47 am, Adam Farley8 wrote:
>> > > >>>>> Hi Serguei, David,
>> > > >>>>>
>> > > >>>>> My turn to apologise for the delay. :)
>> > > >>>>>
>> > > >>>>> I've modified the code as per Serguei's request. Take a 
>> look  and
>> > > > let me
>> > > >>>>> know if this is the sort of thing you were thinking of.
>> > > >>>>>
>> > > >>>>> Webrev: https://urldefense.proofpoint.com/v2/url?
>> > > >>>> 
>> u=http-3A__cr.openjdk.java.net_-7Eafarley_8227021.3_webrev_&d=DwID-
>> > > >>>> g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-
>> > > >>>> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=8Wa8Zdfmvn-
>> > > >>>>
>> > > >
>> > >
>> > 
>> yvzvCAhOyJ_etFblRA4vmLGbKF4aW8PY&s=L19aeFXoR9JIO62QRPFzZObIU8RbhpCtXSvUibD2ISk&e=
>> > > >>>> I'd prefer to see the helper just as a file static function 
>> rather
>> > > > than
>> > > >>>> adding it to the os class.
>> > > >>>>
>> > > >>>> +  * supplied array of arrays of chars (a), where n
>> > > >>>>
>> > > >>>> I assume (a) is meant to refer to the parameter, but you 
>> actually
>> > > > called
>> > > >>>> it arrayarray. I think "a" or "arr" would suffice.
>> > > >>>>
>> > > >>>> Thanks,
>> > > >>>> David
>> > > >>>>
>> > > >>>>> Best Regards
>> > > >>>>>
>> > > >>>>> Adam Farley
>> > > >>>>> IBM Runtimes
>> > > >>>>>
>> > > >>>>>
>> > > >>>>> "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> 
>>  wrote on
>> > > >>>>> 31/07/2019 17:18:05:
>> > > >>>>>
>> > > >>>>>> From: "serguei.spitsyn at oracle.com" 
>> <serguei.spitsyn at oracle.com>
>> > > >>>>>> To: Adam Farley8 <adam.farley at uk.ibm.com>,  David  Holmes
>> > > >>>>>> <david.holmes at oracle.com>
>> > > >>>>>> Cc: serviceability-dev <serviceability-dev at openjdk.java.net>,
>> > > >>>>>> hotspot-dev at openjdk.java.net
>> > > >>>>>> Date: 31/07/2019 17:18
>> > > >>>>>> Subject: Re: RFR: JDK-8227021:  VM fails if any
>> > > > sun.boot.library.path
>> > > >>>>>> paths are longer than JVM_MAXPATHLEN
>> > > >>>>>>
>> > > >>>>>> Hi Adam,
>> > > >>>>>>
>> > > >>>>>> It looks Okay to me.
>> > > >>>>>>
>> > > >>>>>> A couple of minor comments.
>> > > >>>>>>
>> > > >>>>>> https://urldefense.proofpoint.com/v2/url?
>> > > >>>> u=http-3A__cr.openjdk.java.net_-7Eafarley_8227021.
>> > > >>>> 2_webrev_src_hotspot_&d=DwID-g&c=jf_iaSHvJObTbx-
>> > > >>>> 
>> siA1ZOg&r=P5m8KWUXJf-CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=8Wa8Zdfmvn-
>> > > >>>>
>> > > >
>> > >
>> > 
>> yvzvCAhOyJ_etFblRA4vmLGbKF4aW8PY&s=NL6tYuwwDod3DSmj-1ztxAywpO8L52HEyO0wvTR05bs&e=
>> > > >>>>>> share/runtime/os.cpp.frames.html
>> > > >>>>>> 1362       //release  allocated storage  before exiting the vm
>> > > >>>>>> 1363       while (i > 0) {
>> > > >>>>>> 1364           i--;
>> > > >>>>>> 1365           if (opath[i] != NULL)  {
>> > > >>>>>> 1366 FREE_C_HEAP_ARRAY(char,   opath[i]);
>> > > >>>>>> 1367           }
>> > > >>>>>> 1368       }
>> > > >>>>>> 1369 FREE_C_HEAP_ARRAY(char*, opath);
>> > > >>>>>>
>> > > >>>>>> 1377       //release allocated storage before  returning  null
>> > > >>>>>> 1378       while (i > 0) {
>> > > >>>>>> 1379           i--;
>> > > >>>>>> 1380           if (opath[i] != NULL)  {
>> > > >>>>>> 1381 FREE_C_HEAP_ARRAY(char,   opath[i]);
>> > > >>>>>> 1382           }
>> > > >>>>>> 1383       }
>> > > >>>>>> 1384 FREE_C_HEAP_ARRAY(char*, opath);
>> > > >>>>>>
>> > > >>>>>> This duplicated fragments is worth to refactor to a function.
>> > > >>>>>> Also a space is missed at the beginning of the comment.
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>> Thanks,
>> > > >>>>>> Serguei
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>> On 7/31/19 02:01, Adam Farley8 wrote:
>> > > >>>>>> Hi All,
>> > > >>>>>>
>> > > >>>>>> Reviewers requested for the change below.
>> > > >>>>>>
>> > > >>>>>> @David - Agreed. Would you be prepared to sponsor the change?
>> > > >>>>>>
>> > > >>>>>> Bug: https://urldefense.proofpoint.com/v2/url?
>> > > >>>> u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8227021&d=DwID-
>> > > >>>> g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-
>> > > >>>> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=8Wa8Zdfmvn-
>> > > >>>>
>> > > >
>> > >
>> > 
>> yvzvCAhOyJ_etFblRA4vmLGbKF4aW8PY&s=xykJ0KLy9AKWO8zmC0amfR7xxUsvyKEjlf3y7WWOqvE&e=
>> > > >>>>>> Webrev: https://urldefense.proofpoint.com/v2/url?
>> > > >>>> 
>> u=http-3A__cr.openjdk.java.net_-7Eafarley_8227021.2_webrev_&d=DwID-
>> > > >>>> g&c=jf_iaSHvJObTbx-siA1ZOg&r=P5m8KWUXJf-
>> > > >>>> CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=8Wa8Zdfmvn-
>> > > >>>>
>> > > >
>> > >
>> > 
>> yvzvCAhOyJ_etFblRA4vmLGbKF4aW8PY&s=NvIza4VVWG3CiDhmQVmXsghH_4h_c5mFJbHwkCUcut0&e=
>> > > >>>>>> Best Regards
>> > > >>>>>>
>> > > >>>>>> Adam Farley
>> > > >>>>>> IBM Runtimes
>> > > >>>>>>
>> > > >>>>>> P.S. Remembered to add the links this time. :)
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>> David Holmes <david.holmes at oracle.com> wrote on 30/07/2019
>> > > > 03:37:53:
>> > > >>>>>>> From: David Holmes <david.holmes at oracle.com>
>> > > >>>>>>> To: Adam Farley8 <adam.farley at uk.ibm.com>
>> > > >>>>>>> Cc: hotspot-dev at openjdk.java.net, serviceability-dev
>> > > >>>>>>> <serviceability-dev at openjdk.java.net>
>> > > >>>>>>> Date: 30/07/2019 03:38
>> > > >>>>>>> Subject: Re: RFR: JDK-8227021: VM fails if any
>> > > > sun.boot.library.path
>> > > >>>>>>> paths are longer than JVM_MAXPATHLEN
>> > > >>>>>>>
>> > > >>>>>>> Hi Adam,
>> > > >>>>>>>
>> > > >>>>>>> On 25/07/2019 3:57 am, Adam Farley8 wrote:
>> > > >>>>>>>> Hi David,
>> > > >>>>>>>>
>> > > >>>>>>>> Welcome back. :)
>> > > >>>>>>> Thanks. Sorry for the delay in getting back to this.
>> > > >>>>>>>
>> > > >>>>>>> I like .v2 as it is much simpler (notwithstanding freeing
>> > > >> the  already
>> > > >>>>>>> allocated arrays adds some complexity - thanks for fixing 
>> that).
>> > > >>>>>>>
>> > > >>>>>>> I'm still not sure we can't optimise things better for
>> > > > unchangeable
>> > > >>>>>>> properties like the boot libary path, but that's another 
>>  RFE.
>> > > >>>>>>>
>> > > >>>>>>> Thanks,
>> > > >>>>>>> David
>> > > >>>>>>>
>> > > >>>>>> Unless stated otherwise above:
>> > > >>>>>> IBM United Kingdom Limited - Registered in England and 
>> Wales  with
>> > > >>>>>> number 741598.
>> > > >>>>>> Registered office: PO Box 41, North Harbour, Portsmouth,
>> > > >> Hampshire  PO6 3AU
>> > > >>>>> Unless stated otherwise above:
>> > > >>>>> IBM United Kingdom Limited - Registered in England and 
>> Wales  with
>> > > > number
>> > > >>>>> 741598.
>> > > >>>>> Registered office: PO Box 41, North Harbour, Portsmouth,
>> > > >> Hampshire  PO6 3AU
>> > > >>> Unless stated otherwise above:
>> > > >>> IBM United Kingdom Limited - Registered in England and Wales with
>> > > > number
>> > > >>> 741598.
>> > > >>> Registered office: PO Box 41, North Harbour, Portsmouth, 
>> Hampshire PO6
>> > > > 3AU
>> > > > Unless stated otherwise above:
>> > > > IBM United Kingdom Limited - Registered in England and Wales 
>> with number
>> > > > 741598.
>> > > > Registered office: PO Box 41, North Harbour, Portsmouth, 
>> Hampshire PO6 3AU
>> > > >
>> > > >
>> > >
>> >
>> > Unless stated otherwise above:
>> > IBM United Kingdom Limited - Registered in England and Wales with
>> > number 741598.
>> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
>> PO6 3AU
>> Unless stated otherwise above:
>> IBM United Kingdom Limited - Registered in England and Wales with 
>> number 741598.
>> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
>> PO6 3AU
>

More information about the hotspot-dev mailing list