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

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8299665: /proc/self/stat parsing in libmanagement broken by execname with spaces
From:       Kevin Walls <kevinw () openjdk ! org>
Date:       2023-05-26 10:51:57
Message-ID: LgSqfAr-uvIxWiEkCHQQeCaAmT3RHPtYiEe2RJLlR-w=.6442a933-544e-45ee-a221-ef2d416cc8e4 () github ! com
[Download RAW message or body]

On Wed, 24 May 2023 23:48:52 GMT, Serguei Spitsyn <sspitsyn@openjdk.org> wrote:

> > Code that parses /proc/self/stat may be broken if the executable name contains \
> > spaces. 
> > We have code that does this correctly, and code that does not.
> > UnixOperatingSystem.c is Linux-specific OS code, and has vread_statdata which \
> > correctly uses strrchr to find the last ) and parses from there. 
> > OperatingSystemImpl.c the generic, not Linux-specific code, is not OK, as \
> > Java_com_sun_management_internal_OperatingSystemImpl_getCommittedVirtualMemorySize0 \
> > has a problematic fscanf call.  But the problematic fscanf call is \
> > Linux-specific: we use ifdef to create OS-specific implementations. 
> > 
> > Need to change the fscanf of /proc/self/stat to do the same buffered read as \
> > read_ticks in UnixOperatingSystem.c (the Linux-specific code), with strrchr to \
> > read past the ) of the execname. 
> > Moving the actually-Linux-specific getCommittedVirtualMemorySize0 to be with the \
> > other Linux-specific code means it can just use the existing method for accessing \
> > /proc/self/stat. 
> > It does however call throw_internal_error, which is then not accessible.  So that \
> > function can be shared in the libmanagement_ext.h/.c for both uses. 
> > The call to throw_internal_error is working: if I change the code to fail, it \
> > reports a java.lang.InternalError as it would before. 
> > Existing tests still pass: 
> > i.e. test/jdk/com/sun/management/OperatingSystemMXBean/GetCommittedVirtualMemorySize.java
> > 
> 
> Thank you for explanation! Looks good.

Thanks @sspitsyn and @alexmenkov for the reviews!

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14107#issuecomment-1564203867


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