[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    RE: RFR(XS) [TESTBUG] hotspot/jtreg/gtest/GTestWrapper.java fails on AIX and MUSL
From:       "Siebenborn, Axel" <axel.siebenborn () sap ! com>
Date:       2018-04-26 10:15:43
Message-ID: 2a8cbd6aa88a495b81a066d44cd5b6f4 () sap ! com
[Download RAW message or body]

Hi Thomas, 
thanks for your review.
I'll change the comment as you suggested.

Thanks,
Axel

> -----Original Message-----
> From: Thomas Stüfe [mailto:thomas.stuefe@gmail.com]
> Sent: Mittwoch, 25. April 2018 12:37
> To: Siebenborn, Axel <axel.siebenborn@sap.com>
> Cc: Langer, Christoph <christoph.langer@sap.com>; hotspot-runtime-
> dev@openjdk.java.net
> Subject: Re: RFR(XS) [TESTBUG] hotspot/jtreg/gtest/GTestWrapper.java fails
> on AIX and MUSL
> 
> Hi Axel,
> 
> thanks for this fix!
> 
> May I propose a bit clearer comment?
> 
> Instead of
>         // The java launcher sets LD_LIBRARY_PATH or LIBPATH. In such
> a case, we need to prepend
>         // this library path with a path to the gtest libjvm.so.
> 
> Proposal:
> 
> "The GTestWrapper was started using the normal java launcher, which
> may have set LD_LIBRARY_PATH or LIBPATH to point to the jdk libjvm. In
> that case, prepend the path with the location of the gtest library."
> 
> I leave it up to you if you take my comment proposal. No need for a new
> webrev.
> 
> Reviewed.
> 
> Thanks, Thomas
> 
> 
> On Wed, Apr 25, 2018 at 9:09 AM, Siebenborn, Axel
> <axel.siebenborn@sap.com> wrote:
> > Thank you, Christoph!
> >
> > I updated the copyright header.
> >
> > May I have some reviews from official reviewers?
> >
> >
> http://cr.openjdk.java.net/~asiebenborn/jtreg_GTestWrapper/webrev_01/
> >
> > Thanks,
> > Axel
> >
> >> -----Original Message-----
> >> From: Langer, Christoph
> >> Sent: Dienstag, 24. April 2018 09:37
> >> To: Siebenborn, Axel <axel.siebenborn@sap.com>; Mikael Vidstedt
> >> <mikael.vidstedt@oracle.com>
> >> Cc: hotspot-runtime-dev@openjdk.java.net
> >> Subject: RE: RFR(XS) [TESTBUG] hotspot/jtreg/gtest/GTestWrapper.java
> fails
> >> on AIX and MUSL
> >>
> >> Hi Axel,
> >>
> >> looks good. Don't forget to update the current year in the copyright
> header.
> >>
> >> Best regards
> >> Christoph
> >>
> >> > -----Original Message-----
> >> > From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> >> > bounces@openjdk.java.net] On Behalf Of Siebenborn, Axel
> >> > Sent: Montag, 23. April 2018 11:02
> >> > To: Mikael Vidstedt <mikael.vidstedt@oracle.com>
> >> > Cc: hotspot-runtime-dev@openjdk.java.net
> >> > Subject: [CAUTION] RE: RFR(XS) [TESTBUG]
> >> > hotspot/jtreg/gtest/GTestWrapper.java fails on AIX and MUSL
> >> >
> >> > Hi,
> >> > > -----Original Message-----
> >> > > From: Mikael Vidstedt [mailto:mikael.vidstedt@oracle.com]
> >> > > Sent: Freitag, 20. April 2018 20:05
> >> > > To: Siebenborn, Axel <axel.siebenborn@sap.com>
> >> > > Cc: hotspot-runtime-dev@openjdk.java.net
> >> > > Subject: Re: RFR(XS) [TESTBUG]
> hotspot/jtreg/gtest/GTestWrapper.java
> >> > fails
> >> > > on AIX and MUSL
> >> > >
> >> > >
> >> > > I was almost certain that I had already fixed this. And it turned out I
> had.
> >> > > Locally, and then forgotten to actually get the fix pushed :)
> >> > >
> >> > > This is almost exactly the same code I ended up with (although I only
> did
> >> it
> >> > for
> >> > > LD_LIBRARY_PATH).
> >> > >
> >> > > Looks good. Perhaps a short comment in the code explaining what's
> >> going
> >> > > on?
> >> > >
> >> > > Also, small nit: the spaces around the "pb" argument on the last line :)
> >> >
> >> > I added a short comment and removed the superfluous spaces.
> >> >
> >> > New webrev:
> >> >
> >>
> http://cr.openjdk.java.net/~asiebenborn/jtreg_GTestWrapper/webrev_01/
> >> >
> >> > Thanks,
> >> > Axel
> >> >
> >> > >
> >> > > Cheers,
> >> > > Mikael
> >> > >
> >> > >
> >> > > > On Apr 20, 2018, at 4:06 AM, Siebenborn, Axel
> >> > > <axel.siebenborn@sap.com> wrote:
> >> > > >
> >> > > > Hi,
> >> > > >
> >> > > > Webrev:
> >> > >
> http://cr.openjdk.java.net/~asiebenborn/jtreg_GTestWrapper/webrev/
> >> > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8195002
> >> > > >
> >> > > > The test hotspot/jtreg/gtest/GTestWrapper.java fails on AIX,
> >> alpine/musl
> >> > > and probably in all cases RequiresSetenv() in
> >> > > java.base/unix/native/libjli/java_md_solinux.c returns "true".
> >> > > > The problem is, that the java launcher sets the library path
> >> > > (LD_LIBRARY_PATH or LIB_PATH). The test starts an executable, that is
> >> > > dynamically linked against a special gtests libjvm. Due to the inherited
> >> > library
> >> > > path, the wrong libjvm is linked.
> >> > > > The fix is to use ProcessBuilder and set the env variable for
> >> > > LD_LIBRARY_PATH or LIBPATH to the special gtest libjvm.
> >> > > >
> >> > > > Cheers,
> >> > > > Axel
> >

[prev in list] [next in list] [prev in thread] [next in thread]