RFR : 8211326 : add OS user related information to hs_err file

Fri Nov 9 09:08:15 UTC 2018

Hi Matthias,

http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/src/hotspot/os/posix/os_posix.hpp.udiff.html

+  static void print_user_info(outputStream* st, char* buf, size_t buflen);

You can get rid of buf+buflen too, not needed anymore
---
http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/src/hotspot/os/posix/os_posix.cpp.udiff.html

In os::Posix::print_user_info(), I would probably now print it all one
one like, e.g. "uid: 4711, euid: 0, euid: 4711, egid: 4711, umask:
rwxr--r--" - for brevity. But I leave that up to you.
---

The "removing" text does not make sense and can itself be removed :)

--

I spotted a tiny issue which puts the whole print umask into question.
Appearantly, there is no way for umask() to just return the current
value of the file creation mask without changing it. So, that is why
we restore it right afterwards. That is annoying. That leaves a tiny
tiny time window where a different thread could create a file which
would then have permissions all zero'd out.

I am not sure how to deal with that. Could you look around whether
there is another posix API to just look at the umask without changing
it? Otherwise: does anyone think this is a problem?

--

Sorry, turns out simple things are often more complicated than it
first appears...

Cheers, Thomas

On Fri, Nov 9, 2018 at 9:45 AM Baesken, Matthias
<matthias.baesken at sap.com> wrote:
>
> Hi Thomas, thank you  for the remarks .
> I introduced
>
> os::Posix::print_umask
>
> >
> >  But I remember that we did this in our
> > port after some deliberation - there is a risk that name resolving may
> > hang or crash, depending on how exactly user ids are resolved on that  machine.
> >
>
>   ..and removed   the  getpwuid  /   getgrgid   calls (so we only get  the numeric ids in the output and not  the user names / group names)
> New webrev :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.2/
>
>
> Best regards, Matthias
>
>
> > -----Original Message-----
> > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > Sent: Donnerstag, 8. November 2018 21:00
> > To: Baesken, Matthias <matthias.baesken at sap.com>
> > Cc: David Holmes <david.holmes at oracle.com>; Langer, Christoph
> > <christoph.langer at sap.com>; Volker Simonis <volker.simonis at gmail.com>;
> > HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> > Subject: Re: RFR : 8211326 : add OS user related information to hs_err file
> >
> > Hi Matthias,
> >
> > this looks good in general. Two remarks:
> >
> > 1) For convenience reasons, we resolve the user name from its id (and
> > group name etc) with getpwuid. But I remember that we did this in our
> > port after some deliberation - there is a risk that name resolving may
> > hang or crash, depending on how exactly user ids are resolved on that
> > machine. For our port we decided to go with it, since our VM runs
> > usually in very controlled environments, but for the OpenJDK I propose
> > to remove that part from the patch and just print out the numeric
> > values.
> >
> > 2) The part where you print out the mode_t umask as "rwxrwxrwx" is
> > kind of neat. Maybe we could factor that out as an own function in
> > os::Posix? e.g. os::Posix::print_umask(mode_t mask) ?
> >
> > I leave (2) up to you. We also can do this in a follow up patch.
> >
> > Thanks, Thomas
> >
> >
> > On Thu, Nov 8, 2018 at 6:13 PM Baesken, Matthias
> > <matthias.baesken at sap.com> wrote:
> > >
> > > Hello  , after the flag   "-XX:+-ExtensiveErrorReports"   make it into jdk/jdk  ,
> > I created another webrev :
> > >
> > > http://cr.openjdk.java.net/~mbaesken/webrevs/8211326.1/
> > >
> > >
> > > The user info output is now guarded  by ExtensiveErrorReports .
> > >
> > > Please review!
> > >
> > >
> > > Thanks, Matthias
> > >
> > >
> > > > -----Original Message-----
> > > > From: Baesken, Matthias
> > > > Sent: Dienstag, 9. Oktober 2018 16:55
> > > > To: 'Thomas Stüfe' <thomas.stuefe at gmail.com>; David Holmes
> > > > <david.holmes at oracle.com>
> > > > Cc: Langer, Christoph <christoph.langer at sap.com>; Volker Simonis
> > > > <volker.simonis at gmail.com>; HotSpot Open Source Developers
> > <hotspot-
> > > > dev at openjdk.java.net>
> > > > Subject: RE: RFR : 8211326 : add OS user related information to hs_err file
> > > >
> > > > Hi Thomas ,  thanks for filing the CSR .
> > > >
> > > > I am fine with the name proposal   "-XX:+-ExtensiveErrorReports",   but
> > will
> > > > wait  until it is agreed to use this name .
> > > > Then I'll post another webrev  of  8211326  using the flag .
> > > >
> > > > Best regards, Matthias
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > > > > Sent: Montag, 8. Oktober 2018 12:21
> > > > > To: David Holmes <david.holmes at oracle.com>
> > > > > Cc: Baesken, Matthias <matthias.baesken at sap.com>; Langer,
> > Christoph
> > > > > <christoph.langer at sap.com>; Volker Simonis
> > > > <volker.simonis at gmail.com>;
> > > > > HotSpot Open Source Developers <hotspot-dev at openjdk.java.net>
> > > > > Subject: Re: RFR : 8211326 : add OS user related information to hs_err
> > file
> > > > >
> > > > > On Mon, Oct 8, 2018 at 11:50 AM David Holmes
> > > > <david.holmes at oracle.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi Thomas,
> > > > > >
> > > > > > On 8/10/2018 7:43 PM, Thomas Stüfe wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I drafted a CSR:
> > > > > > >
> > > > > > > https://bugs.openjdk.java.net/browse/JDK-8211846
> > > > > > >
> > > > > > > Not sure what to do now, should I open a discussion thread on a
> > > > > > > mailing list? Or just wait for the committee to decide? (I'm doing
> > > > > > > this too rarely..)
> > > > > >
> > > > > > I filled in some of the additional info needed. The text needs to be
> > > > > > completed and the CSR needs to have at least one reviewer added to
> > it.
> > > > > > Then it can be marked "final" and ready for the committee to
> > evaluate.
> > > > > >
> > > > >
> > > > > Thanks! I filled out what was missing and will wait for reviewers.
> > > > >
> > > > > Thanks, Thomas
> > > > >
> > >