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

Pthreads keeps increasing with WASM #11825

Closed
lnxjnky opened this issue Aug 6, 2020 · 9 comments
Closed

Pthreads keeps increasing with WASM #11825

lnxjnky opened this issue Aug 6, 2020 · 9 comments

Comments

@lnxjnky
Copy link

lnxjnky commented Aug 6, 2020

Hi,

I am new to webassembly. I am porting some cross platform code to WebAssembly with WASM=0, USE_PTHREADS=1 and PTHREAD_POOL_SIZE=8.
While running the code in chrome as browser extension, I observe that in chrome task manager the number of dedicated worker keeps increasing and my extension crashes after consuming good amount of memory. It seems all are detach pthreads. Is there a way to investigate what is going wrong.

I am using sdk-fastcomp-tag-1.38.30-64bit version of emsdk.

image

@kripken
Copy link
Member

kripken commented Aug 10, 2020

One way to investigate this could be to look for new Worker in the JS, which is where new workers are created. Then you can add some logging at those times, use the stack to see why they happen, etc. (based on those flags, emscripten should create 8 workers, and then stop, unless you create more using pthread_create etc.)

@lnxjnky
Copy link
Author

lnxjnky commented Aug 12, 2020

Thanks for the guidance. In my native code there are threads getting created using pthread_create, on task completion they get detached calling pthread_detach and exit. I see that PThread.threadExit(result); is invoked on the worker JS side but the worker is not deleted from the list and shows up in chrome task manager. Am I missing something here due to which the cleanup doesn't happen?

@kripken
Copy link
Member

kripken commented Aug 12, 2020

Does calling pthread_exit instead of pthread_detach fix the issue?

If so then it's either a bug in pthread_detach, or the implementation is correct but pthread_detach does not guarantee thread cleanup (I'm not sure either way from reading the docs, but others here know more about pthreads and can answer that).

@lnxjnky
Copy link
Author

lnxjnky commented Aug 12, 2020

I did try to call pthread_exit at the time of task completion it didn't help. I have not tried calling pthread_exit instead of pthread_detach, not sure if that would be really help. Also going through some docs, it seems an implicit call to pthread_exit() is made when a thread completes (when a call to return routine happens).

Also looking at the worker.js code comment below, the exit operation is started even if there is no explicit call to pthread_exit() which is happening in my case.

// The thread might have finished without calling pthread_exit(). If so, then perform the exit operation ourselves.
// (This is a no-op if explicit pthread_exit() had been called prior.)
      if (!Module['noExitRuntime']) PThread.threadExit(result);

@kripken
Copy link
Member

kripken commented Aug 12, 2020

Interesting. Not sure what's going on then.

Note that Workers are not deleted immediately - only the next time the browser does a GC. So seeing a long list of workers may not be a bug. But seeing new workers created unnecessarily would indicate that emscripten is not reusing them, which would be a bug.

@lnxjnky
Copy link
Author

lnxjnky commented Aug 17, 2020

Could you please guide me how should I proceed from here? Can I do something from my side to help? Or do I have to wait for the bug to be created and fixed?

@kripken
Copy link
Member

kripken commented Aug 17, 2020

I think the main options are for you to continue to debug it on your side (did my last comments not help?), or otherwise to create a small testcase you can share with us (without a testcase, there isn't enough information to investigate this).

@lnxjnky
Copy link
Author

lnxjnky commented Aug 20, 2020

On further debugging the generated .js code, I found that the the created thread when detached are not added back to unusedWorkerPool. I modified the generated .js code and did cleanup on thread exit (exit command) and it solved the problem for me.

JS code before: No handling for exit command

worker.onmessage = function (e) 
.
.
else if (d.cmd === "exit") {} else if (d.cmd === "exitProcess") {

Modified code: Cleanup on exit command

worker.onmessage = function (e) 
.
.
else if (d.cmd === "exit") { 
  PThread.freeThreadData(worker.pthread);
  worker.pthread = undefined;
  PThread.unusedWorkerPool.push(worker);
  PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1)
} else if (d.cmd === "exitProcess") {

I am not sure if this is the right change, but it seems workers not getting added to unusedWorkerPool when threads are detached is the problem here. Thoughts?

@kleisauke
Copy link
Collaborator

@lnxjnky Could you try a newer version of Emscripten? v1.38.30 is quite old and does not contain the improvements included in #8286 and #9355 (which should fix the issue that you're describing).

@lnxjnky lnxjnky closed this as completed Aug 31, 2020
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

No branches or pull requests

3 participants