search

natefinch / npipe

A Windows named pipe implementation written in pure Go.
MIT License
297 stars 73 forks source link

Use WaitForMultipleObjectsEx on a separate handle for closing. #19

Closed cmars closed 8 years ago

cmars commented 8 years ago

Note that the appveyor failure is due to pre-existing data races in the v2 branch.

I conducted trial runs of ./cmd/jujud/agent with this proposed branch vs. the current revision specified by Juju. My findings here: http://paste.ubuntu.com/16933543/

With this branch, I had a 10/10 pass rate of the above test. With the version currently used by Juju master, I had a 7/10 pass rate.

To be clear, I don't think this branch is a perfect fix for the issues in npipe, which includes several data races in PipeListener between Accept and Close, as well as some in the tests.

However I do think this pull request could mitigate the immediate issue of LP:#1581157. The separate close handle should allow PipeListener.Close() to preempt waitForCompletion even if the PipeListener.handle has a stale reference.

@natefinch I think this is ready for a review, PTAL.

cmars commented 8 years ago

Nevermind, I ran another trial with this branch and got timeouts. No good :(