spiderfire
spiderfire copied to clipboard
Race condition in the event loop?
This is an odd one, so I'll try my best to explain what I know.
When returning resolved promises from native code, sometimes the promise never actually resolves on the JS side. This happens right now in my implementation of TransformStream, here (but I've seen it happen elsewhere as well, unless I'm mistaken and the issue was something else there): https://github.com/wasmerio/spiderfire/blob/c87d647fcd27da7a962a0310f6d8ee5dc62cdbd4/runtime/src/globals/streams/transform_stream.rs#L246
Basically, on line 274, I'm constructing a resolved promise via this little convenience method:
pub fn new_resolved<'cx>(cx: &'cx Context, value: impl IntoValue<'cx>) -> Promise {
let mut val = Value::undefined(cx);
Box::new(value).into_value(cx, &mut val);
Promise {
promise: TracedHeap::from_local(&cx.root_object(unsafe { CallOriginalPromiseResolve(cx.as_ptr(), val.handle().into()) })),
}
}
When I inspect the promise state on the native side, it is reported as being fulfilled. However, when returning this to the JS side, it never actually resolves (as in, the await xxx
line never finished running). I tried adding reactions to the promise, and those aren't being run either. Please note I've tried using Promise::new
and Promise::resolve
instead of my new_resolved
method and the result was the same.
Now, here's the interesting part: when I run the same code with a debugger, everything suddenly starts to work. It's the same if I add an await sleep(1000)
between the construction of the promise and the await
line. Both of these make me think this may be a race condition (although I don't know how, since the code is single-threaded) or otherwise has something to do with execution order or time. Unfortunately, I'm not very familiar with the event loop, so I haven't been able to investigate that area further.
More context: changing the JS code so that instead of running Promise::new_resolved
on line 274, we run Promise::from
on line 276, the code works without issues. The problem seems to specifically be with promises constructed on the Rust side.
~~The same error happens if I replace Promise::new_resolved
with future_to_promise
.~~
EDIT: using future_to_promise
actually solves the issue locally. Still, since Spidermonkey itself is probably using CallOriginalPromiseResolve
, that does not solve the issue completely.
More context: it actually has nothing to do with the debugger. It runs successfully under debug configuration, but fails under release.
I worked around this issue by switching to opt level 1 for WinterJS, which is the highest opt level where the error doesn't happen. I also tried staring intensely at the event loop code, but didn't find anything wrong with it. Gonna need your help with this one @Redfire75369.
@Arshia001 unfortunately redfire does not have access to laptop for the two weeks or so.
Ah, thanks for the context. I was getting kind of worried since he's usually very responsive. I've investigated this issue as far as I know how, so I'll just wait for him to get back!
I can't seem to replicate the issue. While Promise.resolve
and Promise::new -> Promise::resolve
both don't trigger any event loop jobs, they should still trigger a job when they are await
ed or .then
'd. It's possible that this is an issue with the macrotask implementation which currently runs to completion instead of running one at a time, but I would need more context about where and how this is run for that.
Thanks for looking into this, I'll try to create a repro and post it.