piping binary files sometimes fail

[paolino]

I'm having this random bug , sometimes code succeed, sometimes not. It must be noted that I had to choose 5000 to exploit the randomness of it With 10000 it always fail, with 100 it always succeed. Also substituting "take 5000 fibs" with [0..5000] it always succeed, probably because it's much faster.

This is the console output, for 2 consecutive shots. Notice that faster machines, or different kernels could need a different 5000, or never show the bug.

paolino@paolino-desktop:~$ ./prod | cat |./cons
5000
paolino@paolino-desktop:~$ ./prod | cat |./cons
cons: <stdin>: hLookAhead: invalid argument (Invalid or incomplete multibyte or wide character)

[Baughn]

I'd like to add that - If you write the output of prod to a file, and then read it in, cons doesn't fail. - If you write the output of prod to a file, and read it in through pipe as in 'cat output | ./cons', cons doesn't fail. - If you simplify prod, using [0..] instead of fibs, cons hasn't failed either.

This suggests that the bug is timing-dependent, which doesn't seem plausible on the face of it. However, one way in which pipes are timing-dependent is that the read() system call can return incomplete data when reading from pipes (although not when reading from files, at least on linux). Therefore, it would be useful to consider what happens when read does return incomplete data.

Regardless of the exact circumstances, the bug itself is one that should never happen. Reading bytestrings uses the binary hGetBuf function, and should always ignore encoding; thus, no encoding error should be produced.

[simonmar]

Data.ByteString.Lazy.hGetContents calls hIsEOF which calls hLookAhead, and hLookAhead reads the next Char from the Handle, which does Unicode decoding. Putting stdin into binary mode fixes the problem.

After giving this a bit of thought, I think the best solution is to make hIsEOF not do any decoding, so I'll look into doing that.

[simonmar]

It's not so easy. Bytestring also calls hWaitForInput, which does decoding because it only returns when there is at least one full character to read, not a partial byte sequence. This is the only sensible semantics for hWaitForInput, so I'm afraid we have to take a different approach: I suggest ByteString stops using hWaitForInput (I can fix hIsEOF as described above).

I looked at the code for DB.Lazy.hGetContents, and as far as I can tell it should be possible to use a blocking read rather than the hGetNonBlocking, hIsEOF, hWaitForInput combination that is used now.

[paolino]

Replying to simonmar:

Data.ByteString.Lazy.hGetContents calls hIsEOF which calls hLookAhead, and hLookAhead reads the next Char from the Handle, which does Unicode decoding. Putting stdin into binary mode fixes the problem.

In the program revealing the error, putting stdin in binary mode was not fixing. It is my fault that I coldn't reproduce that behaviour in the given example, which indeed is fixed by this setting.

After giving this a bit of thought, I think the best solution is to make hIsEOF not do any decoding, so I'll look into doing that.

[igloo]

This is now in HEAD and 6.12:

Tue Jan 12 23:03:17 GMT 2010  Simon Marlow <marlowsd@gmail.com>
  * hIsEOF: don't do any decoding (#3808)

[simonmar]

I have attached a patch to bytestring that fixes the problem, waiting for feedback from the bytestring maintainers.

[simonmar]

Ian - Duncan has pushed these patches to  http://darcs.haskell.org/bytestring, could you pull them into GHC HEAD/STABLE please? (re-rolling tarballs, I presume).

[igloo]

Done:

Tue Mar 30 09:16:47 PDT 2010  Ian Lynagh <igloo at earth.li>
  * Update to bytestring 0.9.1.6; fixes #3808

[guest]

This patch breaks current behavior with 6.12.2 and handles derived from network sockets. (At least with OpenSUSE) (I don't know if this is a problem more generally.) The patch description says that hGet returns a short read if it would otherwise block, and only blocks if there is no data to read. However, e.g. happstack, which uses lazy hGetContents over a handle created from a network socket, indeed does block. Substituting the prior version of hGetContents which uses hGetNonBlocking + hWaitForInput + hIsEOF fixes this behavior.

This should either be reverted, or the previous version should be exposed as well, with an appropriate note about differing behaviors. --sclv

[simonmar]

Oh dear, you're right. I must have been dreaming, hGetBuf doesn't behave the way I thought it did. Sorry about this.

[duncan]

Same new issue noticed and reported in ticket #4041.

[simonmar]

Fixed in bytestring:

Wed May 19 16:47:52 BST 2010  Simon Marlow <marlowsd@gmail.com>
  * bump version to 0.9.1.7

Thu May 20 10:03:29 BST 2010  Simon Marlow <marlowsd@gmail.com>
  * ROLLBACK: hGetContents: use hGet instead of hGetNonBlocking + hWaitForInput + hIsEOF
  
  This patch was based on a misconception about the semantics of
  hGetBuf, and caused hGetContents to block more often than it should.
  The change to documentation also caused additional confusion:
  
    http://hackage.haskell.org/trac/ghc/ticket/4041
  
  By rolling back we re-introduce the original bug (GHC bug #3808), but
  That bug has a workaround (put the Handle in binary mode).
  
  In the future we will fix the bug properly using hGetBufSome, see
  
    http://hackage.haskell.org/trac/ghc/ticket/4046

Thu May 20 10:03:38 BST 2010  Simon Marlow <marlowsd@gmail.com>
  * ROLLBACK partly: update docs for hGet, hGetNonBlocking
  
  The change to the docs for hGet was incorrect.  Leaving the change to
  the hGetNonBlocking docs as it is a clarification.

[simonmar]

btw, after merging please leave this ticket open against 6.14.1, as we need to re-implement bytstring's hGetContents in terms of System.IO.hGetBufSome.

[igloo]

Merged.

[simonmar]

The final patch has been pushed upstream to bytestring and to GHC's mirror.

Mon Sep 13 16:10:27 BST 2010  Simon Marlow <marlowsd@gmail.com>
  * add Data.ByteString.hGetSome; use it in Data.ByteString.Lazy.hGetContents
  See GHC ticket #3808

[igloo]

Merged