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

use joinThreads avoiding #80 #88

Closed
wants to merge 1 commit into from
Closed

use joinThreads avoiding #80 #88

wants to merge 1 commit into from

Conversation

bung87
Copy link
Contributor

@bung87 bung87 commented Jul 30, 2023

fix #80

I run helloworld.nim on windows WSL2, start and restart manully, randomly crash by SIGSEGV, even I remove settings param entirely from eventLoop, valgrind tells possibly lost in 3 blocks, and point out proc threadProcWrapDispatch[TArg] , however I don't see any problem there, maybe someone can tell me how this PR works. I just follow the theory "create threads" -> "join threads" like in other language.

@ThomasTJdev
Copy link
Contributor

Nice! I have tested the the PR with a minimal code sample, and it solves the segfault in nim-lang/Nim#21422. I will test it on a larger code base tomorrow.

This PR is actually a revert of the PR #62 where the main thread was included as one of the worker threads. I'm not aware whether this could block other code started on the main thread.

@dom96
Copy link
Owner

dom96 commented Jul 30, 2023

I believe @ajusa fixed some real problems with #62, maybe they can remind us of the context.

But in general, I think the underlying compiler issue should be fixed instead. The fact this change works is likely just luck.

@dom96 dom96 closed this Jul 30, 2023
@ajusa
Copy link
Contributor

ajusa commented Jul 30, 2023

It's been two years so my memory is a little fuzzy. I definitely recall that this was a performance improvement, as before httpbeast would spawn num of CPU + 1 thread for joining all of them, and with the change it now uses the main thread to serve requests. This led to less contention/overhead IIRC.

Based on the PR description there might have been a different issue that was fixed as well, which was calling async code before the server started? Like if you issued an async request right before the server was started, it wouldn't be polled because eventLoop wasn't called in the main thread. I don't recall the specifics anymore, but I want to say that was the actual issue that was solved.

I'm not sure why a segfault is occurring as a result of my changes - it's possible that all we need is a join thread after the main thread runs the event loop, to ensure everything is cleaned up correctly?

@bung87
Copy link
Contributor Author

bung87 commented Jul 30, 2023

if you want runs async you may try ringabout/httpx#27, note your main thread eventloop still block the thread.
the facts are :

  1. there's destroyThread proc in nim under when false block https://github.com/nim-lang/Nim/blob/281016a8022b8e8308e3e578b2c2daa6df4a66a1/lib/std/typedthreads.nim#L153
  2. there's proc close in httpbeast under when false block which not implemented yet.
  3. orc have some issues not solved yet
  4. httpbeast run is not asynchronous procedure
  5. start and restart randomly crash on linux, therefore httpbeast mainly target to unlix like platform.

am not using httpbeast myself, just trying to help someone's problem, so I'll leave this.

@bung87
Copy link
Contributor Author

bung87 commented Jul 30, 2023

explanation by AI:

When a thread is created, it has a default state of being a joinable thread, which means that calling the join method on it will wait for the thread to finish before allowing the program to exit. If you do not call join and the thread is still running when the main thread exits, it will be terminated abruptly and any resources it was using might not be properly released.

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 this pull request may close these issues.

Random segmentation fault when using orc and threads more than 1
4 participants