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

          Sebb created issue -
          Sebb made changes -
          Field Original Value New Value
          Summary ByteArrayOutputStream.count is not always synchronised Inconsistent synchronization of fields
          Description The field ByteArrayOutputStream.count is always accessed in a synchronised block, apart from when the user calls toBufferedInputStream().

          This seems wrong.
          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)
          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
          Niall Pemberton made changes -
          Attachment IO-214-Counting-Stream-Sync.patch [ 12417059 ]
          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
          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.
          Niall Pemberton made changes -
          Link This issue relates to IO-201 [ IO-201 ]
          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
          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
          Niall Pemberton made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 2.0 [ 12312961 ]
          Resolution Fixed [ 1 ]
          Mark Thomas made changes -
          Workflow jira [ 12473630 ] Default workflow, editable Closed status [ 12601884 ]
          Henri Yandell made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development