base: fdReady(): Return only after sycall returns after `msecs` have passed
ClosedPublic

Authored by nh2 on Sep 22 2017, 12:50 PM.

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nh2 created this revision.Sep 22 2017, 12:50 PM
nh2 updated this revision to Diff 14061.Sep 22 2017, 6:44 PM

Rebased

dfeuer accepted this revision.Sep 25 2017, 12:33 AM
dfeuer added a subscriber: dfeuer.
dfeuer added inline comments.
libraries/base/cbits/inputReady.c
183

Did you mean something like if (ready == -1 && errno == EINTR)?

415

Whether we've called what?

This revision is now accepted and ready to land.Sep 25 2017, 12:33 AM
nh2 updated this revision to Diff 14123.Sep 26 2017, 8:27 AM

Rebase

nh2 updated this revision to Diff 14159.Sep 27 2017, 7:32 AM
nh2 marked 2 inline comments as done.

Fix comments

libraries/base/cbits/inputReady.c
183

Yes; I just wanted to simplify it a bit because the exact check isn't relevant for the argument; I'll change to if (EINTR happened).

415

Ah, it is missing.

nh2 updated this revision to Diff 14650.Nov 13 2017, 2:35 PM

Rebased

nh2 updated this revision to Diff 14666.Nov 14 2017, 6:20 AM

Rebased

nh2 updated this revision to Diff 14787.Nov 23 2017, 1:03 PM

Rebase / cleanup

nh2 planned changes to this revision.EditedNov 25 2017, 8:48 AM
nh2 added a subscriber: syd.

With testing from @syd I found that there is an edge case in which this commit doesn't fulfill its promise:

When after remaining = endTime - now;, remaining is < 1 ms, e.g. 999 us, then, we'll call poll(timeout in ms = 0), and thus can return too early (but always less than 1 ms too early).

The same is true when remaining is e.g. 2.1 ms, then we'll pass 2 ms to poll, which may return TIMEOUT, and then we'll have waited for only 2 ms instead of 2.1.

I'll fix it by rounding up the milliseconds.

nh2 updated this revision to Diff 14816.Nov 25 2017, 11:39 AM

Fix discarding of fractional milliseconds (relevant for non-Windows, and Windows character devices like cmd.exe and powershell)

This revision is now accepted and ready to land.Nov 25 2017, 11:39 AM
syd added a comment.Nov 26 2017, 11:46 PM

I've had another sniff test, just to make sure that the +1 doesn't pile up for every rounding error and instead just fixes the rounding error as necessary, which seems to be the case.

This revision was automatically updated to reflect the committed changes.