Tim Ellison wrote: > Could somebody please take a look a the patch below and see if they > believe my explanation for why it fixes a findbugs problem in InputStream? > > The problem: We have a static, skipBuf, that we only use to read junk > data from the stream when skipping a number of bytes. The static is > lazily initialized. It may be null, or too small for the job. The > problem is that another thread could also see null (or a small buffer) > and then set the buffer too small after the calling thread sets it to > /their/ required size. > > The solution: If we take a local copy, localBuf, as a snapshot of > skipBuf before testing it, then even if the skipBuf value is slammed by > another thread we have our local copy. We can also blindly overwrite > any value stored by other threads since they will be doing the same thing. > > So the update of the static is still unsafe, but the usage of it is ok > -- and this is better than making it volatile. > > Agree? > +1 from me. Regards, Oliver > Tim > > > Index: InputStream.java > =================================================================== > --- InputStream.java (revision 820251) > +++ InputStream.java (working copy) > @@ -211,11 +211,16 @@ > } > long skipped = 0; > int toRead = n < 4096 ? (int) n : 4096; > - if (skipBuf == null || skipBuf.length < toRead) { > - skipBuf = new byte[toRead]; > + // We are unsynchronized, so take a local copy of the skipBuf > at some > + // point in time. > + byte[] localBuf = skipBuf; > + if (localBuf == null || localBuf.length < toRead) { > + // May be lazily written back to the static. No matter if it > + // overwrites somebody else's store. > + skipBuf = localBuf = new byte[toRead]; > } > while (skipped < n) { > - int read = read(skipBuf, 0, toRead); > + int read = read(localBuf, 0, toRead); > if (read == -1) { > return skipped; > } > > > -- Oliver Deakin Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU