RFR 9: 8078099 : (process) ProcessHandle should uniquely identify processes

[Peter Levart]

Hi Roger,

I looked at the code briefly and have the following comments:

For ProcessHandleImpl_macosx.c, in method getProcessPids0:

  224     // Get buffer size needed to read all processes
  225     int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_ALL, 0};
  226     if (sysctl(mib, 4, NULL, &bufSize, NULL, 0) < 0) {
  227         JNU_ThrowByNameWithLastError(env,
  228             "java/lang/RuntimeException", "sysctl failed");
  229         return -1;
  230     }
  231
  232     // Allocate buffer big enough for all processes
  233     void *buffer = malloc(bufSize);
  234     if (buffer == NULL) {
  235         JNU_ThrowOutOfMemoryError(env, "malloc failed");
  236         return -1;
  237     }
  238
  239     // Read process info for all processes
  240     if (sysctl(mib, 4, buffer, &bufSize, NULL, 0) < 0) {
  241         JNU_ThrowByNameWithLastError(env,
  242             "java/lang/RuntimeException", "sysctl failed");
  243         free(buffer);
  244         return -1;
  245     }

... the 1st call to sysctl is used to measure the size of the needed 
buffer and the 2nd call fills-in the buffer. The documentation for 
sysctl says:

"     The information is copied into the buffer specified by oldp. The 
size of the buffer is given by the
      location specified by oldlenp before the call, and that location 
gives the amount of data copied after
      a successful call and after a call that returns with the error 
code ENOMEM.  If the amount of data
      available is greater than the size of the buffer supplied, the 
call supplies as much data as fits in
      the buffer provided and returns with the error code ENOMEM.  If 
the old value is not desired, oldp and
      oldlenp should be set to NULL.

      The size of the available data can be determined by calling 
sysctl() with the NULL argument for oldp.
      The size of the available data will be returned in the location 
pointed to by oldlenp.  For some opera-tions, operations,
      tions, the amount of space may change often.  For these 
operations, the system attempts to round up so
      that the returned size is large enough for a call to return the 
data shortly thereafter."

So while not very probable, it can happen that you get ENOMEM from 2nd 
call because of underestimated buffer size. Would it be better to retry 
(re)allocation of buffer and 2nd call in a loop with new estimation 
returned from previous call while the error is ENOMEM?

Another suggestion: What would it be if the buffer size estimation was 
not computed by a call to sysctl with NULL buffer, but was taken from 
the passed-in resulting array size(s). In case the user passes-in arrays 
of sufficient size, you can avoid double invocation of sysctl. Also, if 
ENOMEM happens, you can just return the result obtained and the new 
estimation - no looping in native code. The UNIX, Solaris and Windows 
variants look good.

Regards, Peter

On 06/22/2015 05:10 PM, Roger Riggs wrote:
> Please review changes to ProcessHandle implementation to uniquely 
> identify
> processes based on the start time provided by the OS. It addresses the 
> issue
> of PID reuse.
>
> This is the implementation of the ProcessHandle.equals() spec change in
>     8129344 : (process) ProcessHandle instances should define equals 
> and be value-based
>
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-starttime-8078099/
>
> Issue:
>    https://bugs.openjdk.java.net/browse/JDK-8078099
>
> Thanks, Roger
>