trashable
trashable copied to clipboard
Potential race condition?
I like the idea behind this package.
However, I'm concerned that there's a potential race condition.
Let's consider three Promises. The "original" promise is the one that's running an async operation. The "trashable" promise is the one returned by makeTrashable(original)
. The "dangerous" promise is the one that runs some sort of code we want to avoid running if the trashable Promise was trashed, eg setState. ie:
const original = getSomethingAsync();
const trashable = makeTrashable(original);
const dangerous = trashable.then(x => c.setState({x});
So what if this happens:
-
original
fulfills. - The
then
whichtrashable
defines onoriginal
is called, and it callsresolve(val)
. - If I understand correctly, this does NOT immediately and synchronously call the
x => c.setState({x})
function above. Instead, it enqueues a job onto the Promise Job Queue which says that this should be run later. - Potentially lots of other stuff happens here! I'm not sure if it's legal for JS to do stuff that isn't on the Promise Job Queue here, but for all we know React uses Promises itself and lots of React stuff could happen now.
- Specifically, the React component is unmounted and we trash
trashable
. - Now that job above gets called, and
setState
gets run even though we already trashed the component.
Is this race condition impossible for some reason that I'm missing?
It seems like trashable/trashable-react will work well for nearly all of the times preventing us from doing the actual extra work that we're trying to avoid doing after unmount, but I'm not sure it will 100% of the time prevent us from getting that nasty setState warning.
See https://github.com/hjylewis/trashable/pull/7 for a demonstration. I suppose you could say "well, don't trash from a then
in that particular place" but I'm not sure you can prove that it's impossible for random other code to run there.
Very interesting! Seems like what you are saying is possible. Thanks for explaining it so well.
I'll have to think about this some more and if there's a way to avoid the race (i.e. synchronously call the handler on the trashable promise) without losing garbage collectability.
I think there might be a way of making it so makeTrashable
returns an object with the same interface of a promise (then
and catch
functions) but calls the handlers synchronously on resolve/reject instead of putting it on the Promise Job Queue... In other words, makeTrashable
wraps the promise in a "pseudo-promise" instead of another promise.
Let me know if you have any thoughts!
That sounds scary but maybe it would work.