Deadlock when resolving ExecFuture under Windows
Closed, DuplicatePublic
Actions

Assigned To

Authored By

	Unknown Object (User)
	Nov 19 2013, 3:07 AM

Description

It looks like the recent changes to cslint along side D7599 have revealed a potential deadlock scenario when resolving ExecFuture under Windows.

cslint.exe is halted when trying to write to standard error; looking at the ExecFuture code, it does this:

$this->stdout .= $this->readAndDiscard(
  $stdout,
  $this->getStdoutSizeLimit() - strlen($this->stdout),
  'stdout');
$this->stderr .= $this->readAndDiscard(
  $stderr,
  $this->getStderrSizeLimit() - strlen($this->stderr),
  'stderr');

It also states that there's no nonblocking streams under Windows, so my guess is that the standard error buffer is filled up, while Arcanist is still waiting for the process to terminate so it can read standard output.

I'm not quite sure what we can do about this, other than just ignore standard error on Windows?

Event Timeline

Unknown Object (User) created this task.Nov 19 2013, 3:07 AM

Unknown Object (User) assigned this task to epriestley.

Unknown Object (User) raised the priority of this task from to Needs Triage.

Unknown Object (User) updated the task description. (Show Details)

Unknown Object (User) added a project: Arcanist.

Unknown Object (User) added subscribers: Unknown Object (User), hach-que.

I updated cslint to not output to standard error, but obviously this is still potentially an issue for any other commands we're running under Windows.

T3272 is the only attack on this I have.

I'm just going to merge this into that since I don't have other viable approaches. The only other thing I can come up with is that we could write some sort of sane_io.exe which took a command, executed it, split off separate threads to read its stdio/stderr, and then interleaved them on a single output pipe which we'd split back out on the Phabricator side. This seems like 10x worse than redirecting stderr to a file, though, which is saying something since that's awful.

✘ Merged into T3272.

Deadlock when resolving ExecFuture under WindowsClosed, DuplicatePublicActions

Description

Event Timeline

Deadlock when resolving ExecFuture under Windows
Closed, DuplicatePublic
Actions