| commit | 3c5c0d01ccd60adcaf4602d8cbc1182d7e846317 | |
| author | Daniel Neiter <dneiter@fb.com> | |
| Wed, 27 Jan 2016 23:56:30 +0000 (27 15:56 -0800) | ||
| committer | hhvm-bot <hhvm-bot@fb.com> | |
| Thu, 28 Jan 2016 02:30:40 +0000 (27 18:30 -0800) | ||
| tree | 47c6f9363e2209adf028bfb22f52cd9b4b436c89 | treesnapshot (tar.gz zip) |
| parent | d412c4ef053cba68da068a57a982769b57475a76 | commitdiff |
hhvm: implement awaitable cancelation for external thread events
Summary:
This is new take on cancelling awaitables in order to implement I/O-gating in generic manner (D2776353 was the 1st attempt, which turned out to interract poorly with WriteBarrier and few other corner cases).
This change implements cancellation for ExternalThreadEventWaitHandle. I plan to implement cancellation for SleepWaitHandle in a separate diff, but I don't plan implementing support for any other awaitable types.
From php and asio frameworks point of view, cancelled wait handle is completed with exception. I also took care to support cancellation inside onIOWaitEnter callback (this required minor modification to AsioContext::runUntil).
From external thread's point of view things look exactly the same, as if php request fatalled (ExternalThreadEvent moves to Canceled state and will be deleted during markAsFinished).
If external thread already marked event as completed, cancelation returns false and does nothing (process() will complete the wait handle soon anyway).
Implementation-wise c_ExternalThreadEventWaitHandle::cancel() is mostly copy-pasted from process() and destroyEvent() - I couldn't think of a good way to abstract common code.
To test this change, I've used stream_await(), because it gives me external thread event and is relatively easy to build scriptable wrapper around.
However, my testing exposed issues with FileAwait (returned from stream_await) and CurlMultiAwait (returned from curl_multi_await). Both follow very similar design and suffer from same design issues:
- they may leave dangling eventfd watches after php request thread closed file descriptor (in both cases file descriptors are owned by request threads)
- libevent 1.4 (version, hphp links against) only supports single pending event per read/write per file descriptor on the same event base (i.e. trying to concurrently `await stream_await($fd)` will not work as expected, as only one awaitable would actually receive libevent notification)
Combined with file descriptor reuse and the way libevent 1.4 does book-keeping around pending events, this can cause bad things to happen:
- failing to register libevent watch for fd, because libevent and kernel have diverging views of the world (calling `epoll_ctl(EPOLL_CTL_MOD,...)` on fd, not associated with epfd
- stale libevent notification unregistering libevent watch and thus making new libevent watch to miss notifications
This is an issue (especially for rolling out async curl wider), but this is pre-existing condition (same things can happen due to request fatalling), so let's deal with it separately.
Reviewed By: jano
Differential Revision: D2853264
fb-gh-sync-id: 409bfbc867e0240ccc10449e7e2c201d2e79bc1f
Summary:
This is new take on cancelling awaitables in order to implement I/O-gating in generic manner (D2776353 was the 1st attempt, which turned out to interract poorly with WriteBarrier and few other corner cases).
This change implements cancellation for ExternalThreadEventWaitHandle. I plan to implement cancellation for SleepWaitHandle in a separate diff, but I don't plan implementing support for any other awaitable types.
From php and asio frameworks point of view, cancelled wait handle is completed with exception. I also took care to support cancellation inside onIOWaitEnter callback (this required minor modification to AsioContext::runUntil).
From external thread's point of view things look exactly the same, as if php request fatalled (ExternalThreadEvent moves to Canceled state and will be deleted during markAsFinished).
If external thread already marked event as completed, cancelation returns false and does nothing (process() will complete the wait handle soon anyway).
Implementation-wise c_ExternalThreadEventWaitHandle::cancel() is mostly copy-pasted from process() and destroyEvent() - I couldn't think of a good way to abstract common code.
To test this change, I've used stream_await(), because it gives me external thread event and is relatively easy to build scriptable wrapper around.
However, my testing exposed issues with FileAwait (returned from stream_await) and CurlMultiAwait (returned from curl_multi_await). Both follow very similar design and suffer from same design issues:
- they may leave dangling eventfd watches after php request thread closed file descriptor (in both cases file descriptors are owned by request threads)
- libevent 1.4 (version, hphp links against) only supports single pending event per read/write per file descriptor on the same event base (i.e. trying to concurrently `await stream_await($fd)` will not work as expected, as only one awaitable would actually receive libevent notification)
Combined with file descriptor reuse and the way libevent 1.4 does book-keeping around pending events, this can cause bad things to happen:
- failing to register libevent watch for fd, because libevent and kernel have diverging views of the world (calling `epoll_ctl(EPOLL_CTL_MOD,...)` on fd, not associated with epfd
- stale libevent notification unregistering libevent watch and thus making new libevent watch to miss notifications
This is an issue (especially for rolling out async curl wider), but this is pre-existing condition (same things can happen due to request fatalling), so let's deal with it separately.
Reviewed By: jano
Differential Revision: D2853264
fb-gh-sync-id: 409bfbc867e0240ccc10449e7e2c201d2e79bc1f