loop: add some comment on reset functions#305
Conversation
src/loop.zig
Outdated
|
|
||
| // cancelAll will cancel all future events. | ||
| // The loop will not be usable anymore after cancelAll. | ||
| // It is used only during the deinit. |
There was a problem hiding this comment.
If it's only used during deinit, inline it in deinit?
There was a problem hiding this comment.
Good point, by inlining I realized resetEvents is even useless 🤦
There was a problem hiding this comment.
Now I'm wondering if we should call resetJS and resetZig before the deinit wait.
I think so b/c if the caller didn't we could be blocked by a callback re-creating a setTimeout for example.
There was a problem hiding this comment.
You mean before the while (self.eventsNb(.js) > 0 or self.eventsNb(.zig) > 0) { in deinit?
I think this would also have solved the WPT crash. The Issue with the WPT crash was that wait/run wasn't called on an error (my fix was to add an errdefer js_env.wait()).
It isn't just about blocking forever, in the cast of WPT, the issue was that the callbacks are being executed in deinit after the env was cleared up.
// without `reset`, this will execute any pending callbacks, which references the destroyed env
defer loop.deinit()
// invalidates callbacks in the loop
defer js_env.deinit()
There was a problem hiding this comment.
so yes, I think that's a good idea :)
2 participants
No description provided.