-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
plugin test is flaky on windows #3473
Comments
Will Deno have a standard plugin API? |
I'd like to be able to implement hot module swapping. |
@alexleung it isn't helpful commenting like this on a random issue. I will admit we don't mention the plugin API in the manual though. 😢 But it was merged in #3372. We really need docs for it @bartlomieju @afinch7. |
I've been playing around with this issue a bit. On my PC it fails consistently, and I can't figure out how this test can succeed on non-windows devices. I'm creating a PR with some changes that fix the flakyness, but in turn also hide some error. I hope that the PR will point someone in the right direction to fix it more definitively. In summary there are 2 problems:
|
I know what the problem is here btw: on shutdown, when CoreIsolate is dropped, the resource table gets dropped first, and when the resource table gets dropped the plugin DLL is unloaded and unmapped from memory. Then the ops and op-dispatchers are dropped. When a plugin op or op dispatcher's drop handler (now at an unmapped address) runs, you can guess what happens. I don't know why this is not an issue on linux / mac. Maybe the drop order is different. But it seems more likely that they just don't support unloading SOs/DYLIBs so dropping the plugin too early doesn't matter. |
Maybe we should make async ops own a reference to the resource table (which IIRC is an Rc Refcell).. |
That works until someone writes a plugin that defines it's own resource type with drop handler. I started on a patch making plugin ops have a reference to |
ex1 https://github.com/denoland/deno/pull/3470/checks?check_run_id=340389213
ex2 https://github.com/denoland/deno/pull/3467/checks?check_run_id=340012284
The text was updated successfully, but these errors were encountered: