RFR : 8211326 : add OS user related information to hs_err file
Thomas Stüfe
thomas.stuefe at gmail.com
Fri Nov 9 14:19:36 UTC 2018
Hi Matthias,
On Fri, Nov 9, 2018 at 11:08 AM Baesken, Matthias
<matthias.baesken at sap.com> wrote:
>
> Hi Thomas ,
>
> http://man7.org/linux/man-pages/man2/umask.2.html
>
> says :
>
> It is impossible to use umask() to fetch a process's umask without at the same time changing it. A second call to umask() would then be
> needed to restore the umask. The nonatomicity of these two steps provides the potential for races in multithreaded programs.
>
> Since Linux 4.7, the umask of any process can be viewed via the Umask field of /proc/[pid]/status. Inspecting this field in
> /proc/self/status allows a process to retrieve its umask without at the same time changing it.
>
> So it seems to be possible to just get the umask-info , but only on some OS versions .
>
> Don't know how relevant the small time window is, in case of hs_err file writing probably not so relevant.
> Btw. there is a similar pattern here :
>
> jdk/src/java.prefs/unix/native/libprefs/FileSystemPreferences.c
>
> 94 old_umask = umask(0);
> 95 fd = open(fname, O_WRONLY|O_CREAT, permission);
> 96 result[1] = errno;
> 97 umask(old_umask);
>
>
thanks for checking up on that. I'm fine with that if we only do it in
error reporting.
BestRegards, Thomas
>
> Best regards, Matthias
>
>
> > -----Original Message-----
> > From: Thomas Stüfe <thomas.stuefe at gmail.com>
> > Sent: Freitag, 9. November 2018 10:08
> > 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,
> >
> > 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
> > > > > > >
> > > > >
More information about the hotspot-dev
mailing list