Closed ingebot closed 1 year ago
Comment author: @lgcat76
VMProcess.destroy() may arbitrarily throw InternalError for recently finished processes.
Taking a look to the implementation, and following it to the native part of the implementation of VMProcess.nativeKill() (in java_lang_VMProcess.c) it can be seen that if the value returned by cpproc_kill() is ESRCH (the error code indicated by the kill() system call for non-existing processes) then an InternalError is explicitly thrown. But this can happen under normal conditions if the process is reaped, for example, just before the call to nativeKill().
This can be tested by including an artificial delay in VMProcess.destroy() just before calling nativeKill(), to ease setting the conditions for a process being reaped just before the call:
public synchronized void destroy()
{
if (state == TERMINATED)
return;
try { Thread.sleep(10*1000L); } catch (InterruptedException e) {}
nativeKill(pid);
...
Then a test program like this (tested with jamvm):
import java.io.IOException;
public class TestDestroy
{
public static void main(String[] args) throws IOException, InterruptedException
{
System.out.println("Launching process...");
Process process = Runtime.getRuntime().exec("sleep 5");
System.out.println("Waiting 1s before destroy...");
Thread.sleep(1000);
System.out.println("Calling destroy...");
process.destroy();
}
}
Produces the following output:
Launching process...
Waiting 1s before destroy...
Calling destroy...
Exception in thread "main" java.lang.InternalError: kill(263): No such process
at java.lang.VMProcess.nativeKill(Native Method)
at java.lang.VMProcess.destroy(VMProcess.java:362)
at TestDestroy.main(TestDestroy.java:14)
I just think that the ESRCH error code should be handled as success.
Comment author: @lgcat76
Created attachment 54358 Test code to reproduce the bug
Must be tested with a modified version of the VMProcess class including the artificial delay before nativeKill().
Attached file: TestDestroy.java (application/octet-stream, 244 bytes) Description: Test code to reproduce the bug
Comment author: @lgcat76
Created attachment 54359 Test code to reproduce the bug (modified VMProcess)
Modified VMProcess class with an artificial delay before calling nativeKill() in destroy().
Attached file: VMProcess.java (application/octet-stream, 4104 bytes) Description: Test code to reproduce the bug (modified VMProcess)
Comment author: @lgcat76
Created attachment 54360 Fix for bug
Attached file: classpath-vmprocess-destroy.diff (text/plain, 342 bytes) Description: Fix for bug
Comment author: @guillerodriguez
I just think that the ESRCH error code should be handled as success.
Yes. Throwing an InternalError from a condition that cannot be controlled by user code anyway does not seem right.
Also, the Javadocs have been amended to explicitly state that "If the process is not alive, no action is taken".
Note: this issue was migrated automatically using bugzilla2github
Original bug ID: BZ#108569 From: @lgcat76 Reported version: 0.99 CC: @guillerodriguez