Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-28 Thread Alan Bateman
On 26/06/2015 16:33, Brian Burkhalter wrote: On Jun 26, 2015, at 1:22 AM, Alan Bateman alan.bate...@oracle.com mailto:alan.bate...@oracle.com wrote: That would be better and would deal with the concern with webrev.02. Here’s an updated version incorporating Peter’s ThreadDeath change:

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Alan Bateman
On 25/06/2015 18:58, Brian Burkhalter wrote: Hi Peter, Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean: http://cr.openjdk.java.net/~bpb/8042377/webrev.02/ This

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Peter Levart
On 06/26/2015 09:52 AM, Alan Bateman wrote: On 25/06/2015 18:58, Brian Burkhalter wrote: Hi Peter, Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean:

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Alan Bateman
On 26/06/2015 09:17, Peter Levart wrote: : Quite the contrary, previously, if flush() threw any unchecked exception (ThreadDeath included), it was ignored if out.close() also threw (IOException or any unchecked exception). Yes, this is what I was trying to say. +

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-26 Thread Brian Burkhalter
On Jun 26, 2015, at 1:22 AM, Alan Bateman alan.bate...@oracle.com wrote: That would be better and would deal with the concern with webrev.02. Here’s an updated version incorporating Peter’s ThreadDeath change: http://cr.openjdk.java.net/~bpb/8042377/webrev.03/ Thanks, Brian

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-25 Thread Alan Bateman
On 24/06/2015 20:04, Brian Burkhalter wrote: Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8042377 Patch: http://cr.openjdk.java.net/~bpb/8042377/webrev.00/ The use of try-with-resources in FilteredOutputStream.close() is replaced with explicit handling

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-25 Thread Brian Burkhalter
Hi Peter, Thanks for the code change suggestion. I have modified the patch to use its logic and to revert to the original boolean instance variable instead of the contentious AtomicBoolean: http://cr.openjdk.java.net/~bpb/8042377/webrev.02/ The test passes unchanged. Push will be withheld

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-25 Thread Peter Levart
Hi Brian, On 06/24/2015 09:04 PM, Brian Burkhalter wrote: Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8042377 Patch: http://cr.openjdk.java.net/~bpb/8042377/webrev.00/ The use of try-with-resources in FilteredOutputStream.close() is replaced with

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
On Jun 24, 2015, at 1:54 PM, Chris Hegarty chris.hega...@oracle.com wrote: I remember the initialization problem with java.lang.Thread, but I think the usage here is ok. As you correctly point out, AtomicBoolean is already used in other java.io classes. If HelloWorld starts up then I think

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Bernd Eckenfels
Am Wed, 24 Jun 2015 12:04:58 -0700 schrieb Brian Burkhalter brian.burkhal...@oracle.com: Please review at your convenience. Issue:https://bugs.openjdk.java.net/browse/JDK-8042377 Patch:http://cr.openjdk.java.net/~bpb/8042377/webrev.00/ The use of try-with-resources in

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
I should however note that equivalent usage is already present in the close() methods of FileInputStream, FileOutputStream, and RandomAccessFile which are also in java.io. Perhaps the problem you allude to would be more pertinent to java.lang classes? Brian On Jun 24, 2015, at 1:21 PM, Brian

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Chris Hegarty
On 24 Jun 2015, at 21:28, Brian Burkhalter brian.burkhal...@oracle.com wrote: I should however note that equivalent usage is already present in the close() methods of FileInputStream, FileOutputStream, and RandomAccessFile which are also in java.io. Perhaps the problem you allude to

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
On Jun 24, 2015, at 1:54 PM, Chris Hegarty chris.hega...@oracle.com wrote: The changes themselves look ok to me. Maybe the test could exercise FilterOutputStream directly too? I have updated the patch to exercise FilterOutputStream as well: http://cr.openjdk.java.net/~bpb/8042377/webrev.01/

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Vitaly Davidovich
Right, but that's just to no-op duplicate close() calls which makes sense to me since there's no way to query the output stream to see if it's closed. But that version wasn't trying to deal with (rightfully IMO given this class isn't threadsafe) concurrent calls to close() from different threads.

RE: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Jason Mehrens
If the other I/O classes are using it then it should be safe here then. Just wanted make sure it wasn't creating a new issue. Thanks, Jason From: brian.burkhal...@oracle.com Subject: Re: [9] RFR of 8042377: BufferedWriter and

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Vitaly Davidovich
I must say it's a bit odd seeing an atomic field in an otherwise non-threadsafe class. To be pedantic, what's to prevent close (being called on a different thread from one that allocated the filtered output stream) from seeing inconsistent out field values (assume the stream was published

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
The intent here was to fix a specific problem of exception throwing in close(). Prior to this, re-closing the stream was “protected” by double checked locking. Extending coverage of synchronicity problems to the rest of the class could be addressed in the context of another issue, yet to be

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Vitaly Davidovich
Sorry, where was the double checked locking in FilterOutputStream? I see a plain closed boolean before. sent from my phone The intent here was to fix a specific problem of exception throwing in close(). Prior to this, re-closing the stream was “protected” by double checked locking. Extending

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
To be picky about it, it is not exactly locking but a very similar pattern: 158 public void close() throws IOException { 159 if (closed) 160 return; 161 closed = true; On Jun 24, 2015, at 6:16 PM, Vitaly Davidovich vita...@gmail.com wrote: Sorry, where was the

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread David Holmes
I have to agree with Vitaly here, the use of AtomicBoolean seems out of context in an otherwise thread-unsafe class. Why are we concerned about concurrent closing of a stream that shouldn't be being shared across threads ? On 25/06/2015 11:32 AM, Vitaly Davidovich wrote: Right, but that's

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Ivan Gerasimov
Hi Brian! I'm not sure if it's still needed to have @SuppressWarnings(try) before FilterOutputStream.close(). Sincerely yours, Ivan On 24.06.2015 22:04, Brian Burkhalter wrote: Please review at your convenience. Issue: https://bugs.openjdk.java.net/browse/JDK-8042377 Patch:

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
Hi Ivan, I believe you are correct that this is vestigial. I removed it in my patch but will not post another webrev for this alone. Thanks, Brian On Jun 24, 2015, at 12:34 PM, Ivan Gerasimov ivan.gerasi...@oracle.com wrote: I'm not sure if it's still needed to have @SuppressWarnings(try)

RE: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Jason Mehrens
Brian, Not sure on this but, isn't it a little risky to import AtomicBoolean into such low level class? I vaguely remember there was an issue with using AtomicXXX in java.lang.Thread. Not sure if this case suffers the same fate. Jason From:

Re: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Brian Burkhalter
Jason, Thanks for pointing this out. I am not sure of the answer. Let’s see what the experts have to say. Thanks, Brian On Jun 24, 2015, at 1:03 PM, Jason Mehrens jason_mehr...@hotmail.com wrote: Not sure on this but, isn't it a little risky to import AtomicBoolean into such low level