CallbackHelper has a potential race condition? |
|
Issue description@tedchoc, any thoughts? What's preventing a callback from occurring between a call to waitForCallback() and a subsequent getCallCount()? Would it be better if waitForCallback returned a call count? Alternatively, would it be possible to just write a smart wait() method that only blocks if there hasn't been a callback since construction or since the last call to wait()? That's how most waiters work in C++, it seems more foolproof.
,
Oct 20 2017
I think the typical pattern of CallbackHelper is that you register it before you trigger the condition that could possibly increment it.
Changing waitForCallback to return the current call count seems fine for me, but I'm not sure it will guarantee a fix.
The problem with waitForCallback returning a value is that you might have already observed the event and you won't get another notify call.
I think the best thing to do is get the current call count on the same thread as the code that increment it, so you can verify that you haven't already hit it before calling wait.
Otherwise, you'd have to do something like
try {
int callCount = waitForCallback();
} catch (TimeoutException ex) {
if (conditionIsMet()) {
// continue test
}
}
Returning the call count only solves the case where it will be called repeated and you don't have to worry about the last case. I'd say we are saving just getting the call count on the UI thread before calling wait and if the call count has already incremented then you're good.
,
Oct 20 2017
FWIW, calling it repeatedly is the case we're trying to solve here.
,
Oct 20 2017
> The problem with waitForCallback returning a value is that you might have already observed the event and you won't get another notify call. Isn't that problem solved by registering it before the condition? The purpose of returning the current call count from waitForCallback isn't to avoid missing the first notification; it's to avoid missing notifications that occur after waitForCallback returns. > I think the best thing to do is get the current call count on the same thread as the code that increment it Right, this is only a problem when the test is running on a different thread than the source of the callback. |
|
►
Sign in to add a comment |
|
Comment 1 by paulmiller@chromium.org
, Oct 19 2017