Re: RFR [8024521] (process) Async close issues with Process InputStream

On 25/10/2013 16:59, Martin Buchholz wrote: After rebasing, I propose this change which is shorter and is more obviously correct. I agree, this is simpler (more obvious) so it is a good clean-up. One additional thing may be to add a comment to explain why processExited is synchronized (or alt

Re: RFR [8024521] (process) Async close issues with Process InputStream

Thanks a lot, Martin! I created the Jira issue for your proposal: https://bugs.openjdk.java.net/browse/JDK-8027348. Sincerely yours, Ivan On 25.10.2013 20:06, Martin Buchholz wrote: I propose the test for this be in test/java/lang/ProcessBuilder/ instead of test/java/lang/Runtime/exec and

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 24/10/2013 01:02, Martin Buchholz wrote: I include an improved test, that drops the repro time by an order of magnitude, to around 20 seconds, but still too long and non-portable to make this a well-behaved jtreg test. I suggest checking in this version, except that I would still make this

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 23/10/2013 12:41, Ivan Gerasimov wrote: Thank you Rob for offering the sponsor's help! Here's the exported patch: http://cr.openjdk.java.net/~igerasim/2commit/8024521-jdk8-Async-close-race.patch Alan, I reduced the default time gap to 20 seconds as you suggested. Thanks for dialing it down.

Re: RFR [8024521] (process) Async close issues with Process InputStream

Thank you Rob for offering the sponsor's help! Here's the exported patch: http://cr.openjdk.java.net/~igerasim/2commit/8024521-jdk8-Async-close-race.patch Alan, I reduced the default time gap to 20 seconds as you suggested. Sincerely yours, Ivan On 22.10.2013 23:14, Rob McKenna wrote: Happy

RE: RFR [8024521] (process) Async close issues with Process InputStream

.html [2]: http://openjdk.java.net/jtreg/tag-spec.txt -Original Message- From: Rob McKenna Sent: Friday, October 18, 2013 6:55 AM To: core-libs-dev@openjdk.java.net Subject: Re: RFR [8024521] (process) Async close issues with Process InputStream The question around a manual test is interesting. I&

Re: RFR [8024521] (process) Async close issues with Process InputStream

Happy to vounteer for sponsorship duties. Been following this from the sidelines. -Rob On 22/10/13 20:01, Alan Bateman wrote: On 18/10/2013 17:00, Ivan Gerasimov wrote: Thank you Martin for the references. I've implemented passing the limitation to the application in a similar manner. T

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 18/10/2013 17:00, Ivan Gerasimov wrote: Thank you Martin for the references. I've implemented passing the limitation to the application in a similar manner. The timeout value is passed to the test through the CloseRaceTimeout system property. If the property wasn't set, the default value o

Re: RFR [8024521] (process) Async close issues with Process InputStream

Thank you Martin for the references. I've implemented passing the limitation to the application in a similar manner. The timeout value is passed to the test through the CloseRaceTimeout system property. If the property wasn't set, the default value of 2 minutes is used. When the application cr

Re: RFR [8024521] (process) Async close issues with Process InputStream

The question around a manual test is interesting. I've never seen a precedent for this. I think it should be relatively simple to include a manual test in the repo and make sure it doesn't get run by jtreg, (by altering the regular jtreg directives) but I wasn't aware that it was an option. I

Re: RFR [8024521] (process) Async close issues with Process InputStream

Hello everybody! Here's the updated webrev: http://cr.openjdk.java.net/~igerasim/8024521/3/webrev/ I put the regression test back with some slight modifications. Now, when run in automatic mode, it uses the default time gap of 2 minut

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 17/10/2013 20:13, Ivan Gerasimov wrote: Thank you Alan! Yes, I missed that fact that reading from the recycled file descriptor is actually a problem by itself. Would you please take a look at another updated webrev? It contains another implementation suggested by Paul Sandoz with some m

Re: RFR [8024521] (process) Async close issues with Process InputStream

On Oct 17, 2013, at 9:13 PM, Ivan Gerasimov wrote: > Thank you Alan! > > Yes, I missed that fact that reading from the recycled file descriptor is > actually a problem by itself. > Same here. > Would you please take a look at another updated webrev? > > It contains another implementation

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 18.10.2013 10:13, Martin Buchholz wrote: The latest webrev is missing the test case that was present in earlier revisions. Yes, I removed it from the webrev, following the suggestion posted earlier. I'm going to replace it with an instruction for QA about how to verify the fix. However,

Re: RFR [8024521] (process) Async close issues with Process InputStream

Thank you Alan! Yes, I missed that fact that reading from the recycled file descriptor is actually a problem by itself. Would you please take a look at another updated webrev? It contains another implementation suggested by Paul Sandoz with some minor changes. http://cr.openjdk.java.net/~

Re: RFR [8024521] (process) Async close issues with Process InputStream

On 15/10/2013 16:31, Ivan Gerasimov wrote: Here's the updated webrev with the solution by Paul: http://cr.openjdk.java.net/~igerasim/8024521/1/webrev/ I've also excluded the test from it. Instead, I'm going to write a manual testing instruction for SQE. I've tested the fix and it works as expec

Re: RFR [8024521] (process) Async close issues with Process InputStream

Here's the updated webrev with the solution by Paul: http://cr.openjdk.java.net/~igerasim/8024521/1/webrev/ I've also excluded the test from it. Instead, I'm going to write a manual testing instruction for SQE. I've tested the fix and it works as expected. Sincerely yours, Ivan On 15.10.2013

Re: RFR [8024521] (process) Async close issues with Process InputStream

Thanks Paul! Yes, I like your solution better. I'll test it and send the updated webrev shortly. Sincerely yours, Ivan On 14.10.2013 15:33, Paul Sandoz wrote: On Oct 14, 2013, at 11:36 AM, Ivan Gerasimov wrote: Hi Paul, Thank you for looking at the webrev! On 14.10.2013 12:50, Paul Sando

Re: RFR [8024521] (process) Async close issues with Process InputStream

On Oct 14, 2013, at 11:36 AM, Ivan Gerasimov wrote: > Hi Paul, > > Thank you for looking at the webrev! > > On 14.10.2013 12:50, Paul Sandoz wrote: >> Hi Ivan, >> >> Why do you require closeLock? is it not sufficient leave processExited as is >> and just add: >> >> public synchronized void

Re: RFR [8024521] (process) Async close issues with Process InputStream

Hi Paul, Thank you for looking at the webrev! On 14.10.2013 12:50, Paul Sandoz wrote: Hi Ivan, Why do you require closeLock? is it not sufficient leave processExited as is and just add: public synchronized void close() throws IOException { super.close(); } I want to limit the impact o

Re: RFR [8024521] (process) Async close issues with Process InputStream

Hi Ivan, Why do you require closeLock? is it not sufficient leave processExited as is and just add: public synchronized void close() throws IOException { super.close(); } Plus there was already some logic checking if close was asynchronously called (see BufferedInputStream.close): 372

RFR [8024521] (process) Async close issues with Process InputStream

Hello all! Would you please help review a fix? http://bugs.sun.com/view_bug.do?bug_id=8024521 As of JDK 1.7, ProcessPipeInputStream tries to drain the InputStream when the process exits. However, it is racing with application code that could be closing the inputstream at the same time. This c