Commons IO
  1. Commons IO
  2. IO-214

Inconsistent synchronization of fields

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0
    • Component/s: Streams/Writers
    • Labels:
      None

      Description

      The field ByteArrayOutputStream.count is always accessed in a synchronised block, apart from when the user calls toBufferedInputStream().

      This seems wrong.

      Similarly for the count field in CountingOutputStream.beforeWrite(int n)

        Issue Links

          Activity

          Hide
          Niall Pemberton added a comment -

          I changed ByteArrayOutputStream's constructor:

          http://svn.apache.org/viewvc?view=revision&revision=1002791

          The CountingInput/OutputStream issues are already logged as IO-201

          Closing this as FIXED

          Show
          Niall Pemberton added a comment - I changed ByteArrayOutputStream's constructor: http://svn.apache.org/viewvc?view=revision&revision=1002791 The CountingInput/OutputStream issues are already logged as IO-201 Closing this as FIXED
          Hide
          Niall Pemberton added a comment -

          There is already IO-201 for CountingInputStream/CountingOutputStream

          Show
          Niall Pemberton added a comment - There is already IO-201 for CountingInputStream/CountingOutputStream
          Hide
          Sebb added a comment -

          The currentBuffer and currentBuffer index fields are updated by the private method needNewBuffer(int); however this method is called by the constructor without the benefit of synchronization.

          So if an instance is created in one thread and passed to another existing thread, the fields might not be published correctly.

          Show
          Sebb added a comment - The currentBuffer and currentBuffer index fields are updated by the private method needNewBuffer(int); however this method is called by the constructor without the benefit of synchronization. So if an instance is created in one thread and passed to another existing thread, the fields might not be published correctly.
          Hide
          Sebb added a comment -

          Re: ByteArrayOutputStream - good point, I'd not noticed that.

          Not sure why Findbugs does not detect the inconsistent synch. in InputStream, but it looks like both skip() and afterRead() need to be synchronised.

          Show
          Sebb added a comment - Re: ByteArrayOutputStream - good point, I'd not noticed that. Not sure why Findbugs does not detect the inconsistent synch. in InputStream, but it looks like both skip() and afterRead() need to be synchronised.
          Hide
          Niall Pemberton added a comment -

          The private toBufferedInputStream() method that uses the count field is only ever called by the public static toBufferedInputStream() method and is a local variable only ever accessed within that static method so I can't see how it would ever need to be synchronized.

          Looks like the counting OutputStream issue was introduced recently in IO-211 - and the same applies to counting InputStream

          Show
          Niall Pemberton added a comment - The private toBufferedInputStream() method that uses the count field is only ever called by the public static toBufferedInputStream() method and is a local variable only ever accessed within that static method so I can't see how it would ever need to be synchronized. Looks like the counting OutputStream issue was introduced recently in IO-211 - and the same applies to counting InputStream

            People

            • Assignee:
              Unassigned
              Reporter:
              Sebb
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development