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 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.

sent from my phone
On Jun 24, 2015 9:26 PM, "Brian Burkhalter" <brian.burkhal...@oracle.com>
wrote:

To be picky about it, it is not exactly locking but a very similar pattern:

To be picky about it it isn't locking at all - period! Nor is there a double-check.

David
-----

  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 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 coverage of synchronicity problems to the rest
of the class could be addressed in the context of another issue, yet to be
filed, should this be deemed desirable.

On Jun 24, 2015, at 5:34 PM, Vitaly Davidovich <vita...@gmail.com>
wrote:

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 unsafely)? Readers/maintainers of this class may question why
this sole bit is covered via an atomic but rest of class isn't.


Reply via email to