Moved to libraries@ u.stenzel:
Greetings,
I was trying to understand the magic inside Data.Binary, and found two somewhat suspicious uses of inlinePerformIO, which imho has a far too innocuous name:
| toLazyByteString :: Builder -> L.ByteString | toLazyByteString m = S.LPS $ inlinePerformIO $ do | buf <- newBuffer defaultSize | return (runBuilder (m `append` flush) (const []) buf)
Why is this safe? Considering the GHC implementation of IO, isn't there a real danger that 'newBuffer defaultSize' is floated out and therefore every invocation of 'toLazyByteString' starts out with the same buffer? Isn't that exactly the reason why unsafePerformIO and runST are declared NOINLINE?
Yes, that breaks the first rule of inlinePerformIO: * don't use it around code that directly allocates. I've submitted a patch to use unsafePerformIO here.
The other occurence is:
| unsafeLiftIO :: (Buffer -> IO Buffer) -> Builder | unsafeLiftIO f = Builder $ \ k buf -> inlinePerformIO $ do | buf' <- f buf | return (k buf')
which might be safe, since 'f buf' cannot float out of the lambda which binds 'buf', but still, all this stuff is inlined, something constant might get plugged in the place of buf and the result might be floated out to give us an even harder to find Heisenbug.
And this use is indeed also critical for performance, its how we use direct cascades of FFI writes to fill buffers in a an ST style.
Am I missing something and this is actually safe? If not, what can be
We belive the latter use is safe, but it's been 6 months since I've thought about this. Duncan, you've thought a bit more about this recently. Any rules we can use here?
done to avoid such errors? I'd really hate to find building blocks that crumble under pressure in standard libraries...
-- Don