RFR 9: 8077350 Process API Updates Implementation Review (Due 4/23)

[Roger Riggs]

Hi Thomas,

Good point and more robust.

Thanks, Roger


On 4/21/2015 11:49 AM, Thomas Stüfe wrote:
> Hi Roger,
>
> small remark, I see you at a number of places using the same pattern 
> when reading information from /proc:
>
>  342     ret = fread(&psinfo, 1, (sizeof psinfo), fp);
>  343     fclose(fp);
>  344     if (ret < 0) {
>  345         return;
>  346     }
>
> A better way would be to check for the whole size you intended to read:
>
>  342     ret = fread(&psinfo, 1, (sizeof psinfo), fp);
>  343     fclose(fp);
>  344     if (ret < sizeof(psinfo)) {
>  345         return;
>  346     }
>
> /proc may get stale while you read it, so make sure fread actually 
> filled the whole output structure, otherwise you may act on partially 
> random memory content. This is not theoretical :) I had exactly this 
> problem on Solaris when reading structures (in our cases 
> /proc/self/lstatus) from /proc.
>
> Kind Regards, Thomas
>
>
>
>
>
> On Fri, Apr 17, 2015 at 9:12 PM, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto:Roger.Riggs at oracle.com>> wrote:
>
>     The webrev for ProcessAPI updates has been updated to reflect
>     recent comments.
>     Please  review and comment by April 23rd.
>
>     The updates include:
>      - Renaming Process/ProcessHandle supportsDestroyForcibly to
>     supportsNormalTermination
>        and updating related descriptions
>     - ProcessHandle.destroy/destroyForcible descriptions have more
>     consistent descriptions
>     - ProcessHandle.destroy now returns ProcessHandle to enable fluent
>     use.
>     - Corrected description of default implementation ProcessHandle.onExit
>
>     The API doc: http://cr.openjdk.java.net/~rriggs/ph-apidraft/
>     <http://cr.openjdk.java.net/%7Erriggs/ph-apidraft/>
>
>     The webrev: http://cr.openjdk.java.net/~rriggs/webrev-ph
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-ph>
>
>     Issue: JDK-8077350
>     <https://bugs.openjdk.java.net/browse/JDK-8077350> Process API
>     Updates Implementation
>
>     The code is in the jdk9 sandbox on branch JDK-8046092-branch.
>
>     Two issues raised have been filed to be fixed after the current
>     commit:
>      - JDK-8078099 <https://bugs.openjdk.java.net/browse/JDK-8078099>
>     (process) ProcessHandle should uniquely identify processes
>      - JDK-8078108 <https://bugs.openjdk.java.net/browse/JDK-8078108>
>     (process) ProcessHandle.isAlive should be robust
>
>