Builder: avoid unsound buffer reuse (#690) (#691)

`toLazyByteString :: Builder -> LazyByteString` had a race condition that could
generate wrong results if two threads concurrently evaluated the result.  This
bug was introduced in #581 (5c4d236) and first
present in release 0.11.5.0 (as 0c030bb).

Due to the use of `unsafeDupablePerformIO` for performance, it is critical that
any IO actions executed when running a `Builder` can be interrupted or executed
multiple times.  In principle, filling a buffer is safe provided the buffer is
used only once and the same bytes are written each time. However, `wrapChunk` in
`buildStepToCIOS` would re-use a buffer in the trimming case after copying its
contents to produce a new trimmed chunk. This is safe when run in a single
thread, but if two threads simultaneously execute the code, one of them may
still be copying the contents while the other starts overwriting the buffer.

This patch fixes `wrapChunk` to unconditionally allocate a new buffer after
trimming, rather than re-using the old buffer. This will presumably come at a
slight performance cost for builders inserting many trimmed chunks.

Author: adamgundry

Data/ByteString/Builder/Internal.hs

@@ -1196,7 +1196,7 @@ buildStepToCIOS (AllocationStrategy nextBuffer bufSize trim) =
                 -- Checking for empty case avoids allocating 'n-1' empty
                 -- buffers for 'n' insertChunkH right after each other.
                 if isEmpty
-                  then fill nextStep (Buffer fpbuf (BufferRange pbuf pe))
+                  then fill nextStep buf
                   else do buf' <- nextBuffer (Just (buf, bufSize))
                           fill nextStep buf'
 
@@ -1208,9 +1208,9 @@ buildStepToCIOS (AllocationStrategy nextBuffer bufSize trim) =
           | trim chunkSize size = do
               bs <- S.createFp chunkSize $ \fpbuf' ->
                         S.memcpyFp fpbuf' fpbuf chunkSize
-              -- Instead of allocating a new buffer after trimming,
-              -- we re-use the old buffer and consider it empty.
-              return $ Yield1 bs (mkCIOS True)
+              -- It is not safe to re-use the old buffer (see #690),
+              -- so we allocate a new buffer after trimming.
+              return $ Yield1 bs (mkCIOS False)
           | otherwise            =
               return $ Yield1 (S.BS fpbuf chunkSize) (mkCIOS False)
           where