haskell / bytestring

An efficient compact, immutable byte string type (both strict and lazy) suitable for binary or 8-bit character data.
http://hackage.haskell.org/package/bytestring
Other
291 stars 141 forks source link

Builder: avoid unsound buffer reuse #691

Closed adamgundry closed 2 months ago

adamgundry commented 3 months ago

Fixes #690.

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 (5c4d23670e32967ad615699b5262e9cba4daccb7) and first present in release 0.11.5.0 (as 0c030bb63999117a6b1cb1275245a156313c0e49).

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.

Bodigrim commented 1 month ago

This is meant to be both 0.11.5.4 and 0.12.2.0, but GitHub allows only one milestone.