Skip to content
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

AsyncWorkers prevent Node.js from exiting #25

Closed
toyobayashi opened this issue Jan 17, 2023 · 6 comments · Fixed by #26
Closed

AsyncWorkers prevent Node.js from exiting #25

toyobayashi opened this issue Jan 17, 2023 · 6 comments · Fixed by #26

Comments

@toyobayashi
Copy link
Owner

toyobayashi commented Jan 17, 2023

@toyobayashi toyobayashi changed the title AsyncWorkers prevent Node.js from existing AsyncWorkers prevent Node.js from exiting Jan 17, 2023
@RReverser
Copy link
Contributor

Oh nice, I thought I'd eventually implement this myself but cool that you already did 😅

I think it's still better at some point to implement as much as possible upstream in Emscripten though - in particular, there is no reason not to add the refcounter to emscripten_keepalive_runtime_{push,pop} which you already used.

@toyobayashi
Copy link
Owner Author

Agree with you. But I'm not familiar with how to contribute emscripten codebase, and it may be not easy to pull some dependencies due to the network environment limitation in China mainland. Could you help take this to emscripten?

In addition, is there a better way than using setInterval?

@RReverser
Copy link
Contributor

and it may be not easy to pull some dependencies

I don't think it uses any special dependencies outside of what emsdk has (Python and Node.js + some modules). Not sure if any of those modules are blocked in China, but worth a try. But yeah, I can try and can help myself too.

In addition, is there a better way than using setInterval?

Oh... you're using setInterval? Where? That definitely shouldn't be necessary.

@toyobayashi
Copy link
Owner Author

class NodejsWaitingRequestCounter {
private timer: any = undefined
private count: number = 0
public increase (): void {
if (this.count === 0) {
this.timer = setInterval(() => {}, 1e8)
}
this.count++
}
public decrease (): void {
if (this.count === 0) return
if (this.count === 1) {
clearInterval(this.timer)
this.timer = undefined
}
this.count--
}
}

I didn't think of any other way at the time. All the emnapi async worker threads should be unreffed, but there must be a strong ref to prevent nodejs from exiting the event loop.

@RReverser
Copy link
Contributor

Yeah I was thinking on it before for the mentioned contribution, and I thought I'd go with MessageChannel instead. MessagePort is probably the most lightweight way to .ref()/.unref() something without ever accidentally triggering the handler or having to subscribe to the timer.

That is, just do refHandle = new MessageChannel().port1 and use that for all .ref()/.unref() operations.

@toyobayashi
Copy link
Owner Author

Thanks, learned from you again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants