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.

Sign up for GitHub

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

Jump to bottom

BorrowError on a server with a huge amount of tasks #60

Closed
nullchinchilla opened this issue Oct 16, 2023 · 2 comments · Fixed by #61
Closed

BorrowError on a server with a huge amount of tasks #60

nullchinchilla opened this issue Oct 16, 2023 · 2 comments · Fixed by #61

Comments

@nullchinchilla
Copy link

I am seeing my server crash very occasionally like this:

Oct 15 17:21:49 sd-177498 geph4-exit-start[26711]: thread 'sscale-wkr-c' panicked at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-executor-1.5.4/src/lib.rs:950:49:
Oct 15 17:21:49 sd-177498 geph4-exit-start[26711]: already mutably borrowed: BorrowError
Oct 15 17:21:49 sd-177498 geph4-exit-start[26711]: note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@notgull
Copy link
Member

notgull commented Oct 16, 2023

I'm starting to think that the new thread local code isn't worth the trouble, as it's added a new layer of complexity to the executor.

notgull added a commit that referenced this issue Oct 17, 2023
@notgull
Remove the thread-local executor optimization
83cd8b3 
This was added in #37 as an optimization, but has since lead to many bugs. See
the issues #53, #57 and #60 for more information. I do not have the bandwidth
to address all of these bugs, so I'm taking the path of least resistance by
just removing the problematic code.

CLoses #53, #57 and #60

Signed-off-by: John Nunley <[email protected]>
@notgull notgull mentioned this issue Oct 17, 2023
Merged
notgull added a commit that referenced this issue Oct 17, 2023
@notgull
m: Remove the thread-local executor optimization
8a0832c 
This was added in #37 as an optimization, but has since lead to many bugs. See
the issues #53, #57 and #60 for more information. I do not have the bandwidth
to address all of these bugs, so I'm taking the path of least resistance by
just removing the problematic code.

CLoses #53, #57 and #60

Signed-off-by: John Nunley <[email protected]>
@nullchinchilla
Copy link
Author

Yeah, I think keeping async-executor simple is the right solution, especially because more complex executors with any sort of "thread-local" logic are extremely hard to make compatible with nested executors. Typically I just use smolscale as my global executor but smol::Executor for local executors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove the thread-local executor optimization smol-rs/async-executor
2 participants
@notgull @nullchinchilla