Page MenuHomePhabricator

Support non-blocking reads for ExecFuture on Windows

Authored by hach-que on Feb 5 2015, 3:27 AM.
Referenced Files
Unknown Object (File)
Sun, Aug 7, 4:41 AM
Unknown Object (File)
Wed, Aug 3, 8:36 PM
Unknown Object (File)
Tue, Aug 2, 11:19 PM
Unknown Object (File)
Sat, Jul 30, 6:38 AM
Unknown Object (File)
Fri, Jul 29, 2:00 PM
Unknown Object (File)
Thu, Jul 28, 7:23 PM
Unknown Object (File)
Sun, Jul 24, 1:13 PM
Unknown Object (File)
Sun, Jul 24, 7:23 AM



This enables non-blocking reads on Windows for ExecFuture by redirecting the process's standard output and error to temporary files. We then use stream_select on the open files to determine when there is new data to read.

This is primarily to enable setUpdateInterval to work on FutureIterator when running under Windows, which is essential when performing arc unit --everything with lots of tests.

Test Plan

Ran arc diff and arc unit --everything on a project with lots of tests. Saw everything behave correctly and saw setUpdateInterval work as it does on Linux.

Diff Detail

rPHU libphutil
Lint Not Applicable
Tests Not Applicable

Event Timeline

hach-que retitled this revision from to Support non-blocking reads for ExecFuture on Windows.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
hach-que edited edge metadata.
  • Remove commented code

Some inlines. Since this mechanism is certain to be fraught with peril, can we reasonably have an ExecFuture opt in to it for now until we're more confident it works properly? Then only your stuff will break if we hit further Windows weirdness. For example, something like setUseWindowsFileStreams(true).


This seems completely crazy -- do feof(), fseek(), ftell(), etc., not work? I'd expect them to work properly since these are standard files.


$real_length is only written to and never read, so something is awry in any case.


These objects are local and not preserved, so I would expect that we'll try to destroy the temporary files immediately upon exiting this function, and fail. In net, I'd expect that we will never remove these files. Do these files get removed?

Roughly, I'd expect that you need to save these to $this->something. That might also resolve the unlink issue on its own.

We should make every reasonable effort to unlink these files.

The default permissions of TempFile also aren't as locked-down as they could be, but it seems like these files should be as tightly locked as possible. Not sure if we can reasonably do anything or what chmod() translates to under Windows, though.


These calls should be tested for errors.


As above.

hach-que edited edge metadata.
  • Changes from code review feedback

Remove stream_select as things appear to work without it

Uh, don't derp on the arc diff

epriestley edited edge metadata.
epriestley added inline comments.

These variables are unused.





This revision is now accepted and ready to land.Feb 9 2015, 2:17 PM
hach-que edited edge metadata.

Update for code review

This revision was automatically updated to reflect the committed changes.