text icon indicating copy to clipboard operation
text copied to clipboard

`hPutStrLn` in multiple threads

Open worm2fed opened this issue 2 years ago • 8 comments

Function to print text with newline performs it as two separate actions - first is to print message and second - to print newline symbol. This way there is issues with computations in multiple threads, so newline symbol sometimes printed in wrong place.

Possible solution here is to redefine hPutStrLn like this:

hPutStrLn :: Handle -> Text -> IO ()
hPutStrLn h t = hPutStr h $ t <> '\n'

Not sure what is the reason of current implementation.. I found that issue when used co-log and I've provided PR with a fix (https://github.com/co-log/co-log/pull/243), but the source problem is text library.

worm2fed avatar Jun 06 '22 09:06 worm2fed

I'd rather document that none of IO actions in Data.Text.IO are thread-safe. Making them thread-safe would add an overhead for people who don't need that.

EDIT: And e.g. if people need to output two lines, thread-safe hPutStrLn won't help. hPutStrLn $ firstLne <> "\n" <> secondLine is inefficient (performs unnecessary copying).

phadej avatar Jun 06 '22 09:06 phadej

@phadej yes, such a note can be useful to avoid confusion

worm2fed avatar Jun 06 '22 10:06 worm2fed

bytestring defines hPutStrLn as below:

hPutStrLn :: Handle -> ByteString -> IO ()
hPutStrLn h ps
    | length ps < 1024 = hPut h (ps `B.snoc` 0x0a)
    | otherwise        = hPut h ps >> hPut h (B.singleton 0x0a) -- don't copy

This is still not atomic, but much less annoying in practice. I think doing the same trick in text + expanding documentation is a reasonable way to proceed.

Bodigrim avatar Jun 06 '22 20:06 Bodigrim

I initially thought _Why not use hPutBuilder _

hPutBuilder h (fromText ps <> fromText "\n")

but then found that the text package doesn't have an equivalent - is there a good reason for this?

ghost avatar Jun 21 '22 06:06 ghost

bytestring defines hPutStrLn as below:

hPutStrLn :: Handle -> ByteString -> IO ()
hPutStrLn h ps
    | length ps < 1024 = hPut h (ps `B.snoc` 0x0a)
    | otherwise        = hPut h ps >> hPut h (B.singleton 0x0a) -- don't copy

@Bodigrim but length can be expensive O(n)

worm2fed avatar Jun 21 '22 07:06 worm2fed

We don't need Text.length in Chars, we need the length of the array, which should be O(1) right?

ghost avatar Jun 22 '22 01:06 ghost

@axman6-da hPutBuilder is a sensible addition, but I'm not sure it provides guarantees of atomicity even in bytestring.

Bodigrim avatar Jun 22 '22 01:06 Bodigrim

This seems to be a duplicate of https://github.com/haskell/text/issues/242.

It would be nice if someone has energy if not fix it completely then at least alleviate in typical scenarios.

Bodigrim avatar Feb 21 '24 01:02 Bodigrim