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

[Thomas Stüfe]

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> 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/
>
> The webrev: http://cr.openjdk.java.net/~rriggs/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
>