On Wed, 17 Mar 2021 15:18:41 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> test/jdk/java/lang/ProcessBuilder/Basic.java line 2183:
>> 
>>> 2181:                 latch.await();
>>> 2182:                 Thread.sleep(100L);     // Wait for child 
>>> initialization to settle
>>> 2183: 
>> 
>> Hi Roger,
>> Shouldn't the code that follows this sleep already be enough to wait for 
>> child thread to get a lock on the stream?
>> 
>>                 Thread.sleep(100L);     // Wait for child initialization to 
>> settle
>> 
>>                 if (s instanceof BufferedInputStream) {
>>                     // Wait until after the s.read occurs in "thread" by
>>                     // checking when the input stream monitor is acquired
>>                     // (BufferedInputStream.read is synchronized)
>>                     while (!isLocked(s, 10)) {
>>                         Thread.sleep(100);
>>                     }
>>                 }
>>   
>> 
>> From 1st glance I would conclude that the 1st sleep is unnecessary. Is there 
>> a platform where Input/Error stream is not a BufferedInputStream? Or is it 
>> calling Process.destory() immediately after the child thread enters 
>> BufferedInputStream.read synchronized method perhaps too early to trigger 
>> appropriate exception in the child thread? In that case the additional sleep 
>> should be added after the while(!isLocked... loop.
>
> That loop is checking that the Thread (in the parent) reading from the child 
> is in the correct state (blocked).  On Windows, it is a BufferedInputStream.
> 
> But it does not indicate anything about the state of the child process.
> From the scant information from previous failures, it appears that the the 
> creation of some thread (not a java thread) in the child has failed.
> The additional time is to avoid destroying the child while it is still 
> initializing.

The failures happened in tiers 6 and 8. The system may be overloaded so even 
100ms may not be enough for the child process to start sleeping. From the error 
log, the child process tried to spawn a thread (probably one of those usually 
started during VM bootstrap) at around 118ms

[0.118s][warning][os,thread] Failed to start thread - _beginthreadex failed 
(EACCES) for attributes: stacksize: default, flags: 

The test runs 4 times. Each time it checks only STDOUT or STDERR, but not both. 
So I think we can use the other stream to signal to the main process that the 
child process is ready. That would be more reliable than an arbitrary wait time.

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

PR: https://git.openjdk.java.net/jdk/pull/3049

Reply via email to