Implement newCrossThreadPromiseFulfiller().
#1174
Conversation
kentonv
force-pushed
the
kenton/xthread-fulfiller
branch
from
7cc8bdd
to
96d5921
Compare
|
(No changes since last night, just squashed some commits now that they're verified to work.) |
| bool shouldFulfill() { return obj != nullptr; } | ||
|
|
||
| template <typename T> | ||
| XThreadPafImpl<T>* getTarget() { return static_cast<XThreadPafImpl<T>*>(obj); } |
There was a problem hiding this comment.
Would this be a use case for kj::downcast<T>()? (Fine with me either way.)
There was a problem hiding this comment.
Maybe. In this case, you can easily verify from surrounding code that the downcast is safe, so I'm comfortable skipping checking entirely. I like to use downcast more when it's part of an API's contract that you're supposed to pass in a specific type, so it's checking that the calling code didn't screw up.
There was a problem hiding this comment.
Ah, like a require vs an assert.
| template <typename T> | ||
| PromiseFulfillerPair<T> newCrossThreadPromiseAndFulfiller(); | ||
| // Like `newPromiseAndFulfiller()`, but the fulfiller is allowed to be invoked from any thread, | ||
| // not just the one that called this method. |
There was a problem hiding this comment.
It might be worth mentioning that both the promise's and the fulfiller's destructors will lock the construction thread's executor. I wrote a deadlock due to not considering the similar behavior in executeAsync()'s returned promise at one point.
There was a problem hiding this comment.
Hmm, I don't understand how this could lead to deadlock. For deadlock, you have to have two mutexes locked in opposite orders. We shouldn't be locking any other mutexes while holding an executor lock -- if we are, then that's a bug that needs fixing.
There was a problem hiding this comment.
As discussed in chat, I was remember something slightly different:
- Thread A locks Mutex A.
- Thread B blocks waiting for Mutex A.
- Thread A waits on Thread B's executor state to change, but it never does.
It looks like this doesn't suffer from the same problem -- the XThreadPaf's disposer does perform a wait on its own thread's executor, but this wait will always succeed because it's just waiting for FULFILLING to become FULFILLED.
So, never mind me.
| // MSVC's atomic intrinsics are weird and different, whereas the C++ standard atomics match the GCC | ||
| // builtins -- except for requiring the obnoxious std::atomic<T> wrapper. So, on MSVC let's just | ||
| // #define the builtins based on the C++ library, reinterpret-casting native types to | ||
| // std::atomic... this is cheating but ugh, whatever. |
There was a problem hiding this comment.
Your sin has been recorded in the ledger.
For use cases that fit, this should be significantly easier to use, and also perform better, than `Executor` has been.
kentonv
force-pushed
the
kenton/xthread-fulfiller
branch
from
44a5d00
to
1e8868a
Compare
|
Rebased on master, no other changes. Previous MacOS failure was a timer test flake. Gonna merge. |
For use cases that fit, this should be significantly easier to use, and also perform better, than
Executorhas been.