Use our own Thread struct instead of std::thread #8
No reviewers
Labels
No labels
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: lix-project/nix-eval-jobs#8
Loading…
Reference in a new issue
No description provided.
Delete branch "own-thread"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
We'd highly prefer using
std::thread
here; but this won't let us configure the stack size. macOS uses 512KiB size stacks for non-main threads, and musl defaults to 128k. While Nix configures a 64MiB size for the main thread, this doesn't propagate to the threads we launch here. It turns out, running the evaluator under an anemic stack of 0.5MiB has it overflow way too quickly. Hence, we have our own custom Thread struct.Requires a bit of C++ cleanup, but seems to work; and allows more complicated evaluations to run on darwin(-arm64) reliably.
@ -102,0 +104,4 @@
// While Nix configures a 64MiB size for the main thread, this doesn't propagate to the
// threads we launch here. It turns out, running the evaluator under an anemic stack of
// 0.5MiB has it overflow way too quickly. Hence, we have our own custom Thread struct.
struct Thread {
maybe our brain is constantly having too much of a crab rave, but this particular struct feels very risky lifetimes wise: if you ever destroy the backing memory without joining the thread first, you blow up the closure of the std::function, unless they miraculously made std::function a shared pointer or something.
thinking out loud: it seems potentially desirable to put the entire structure into a shared_ptr that itself internally retains a reference to as long as the thread execution inside lives. and even that feels potentially like it's not strictly correct at destruction time from an orphaned thread.
orrr on thread init you just move the std function into a stack variable, but hmmm, you actually would want to hand it something that it only has to destruct from the outer scope, to be sound against the thread taking a while to create and it getting destroyed in the mean time. yeah ok. so you would change the context type you give the inner thread to just a unique_ptr of std::function, which you .release() when you create the thread, then reconstitute into a unique_ptr inside init().
that should be trivially sound, yeah.
an alternative you could do to be suitably annoying about this is to just join the thread in the destructor, which probably would have to be able to throw. oh no how does this deal with exceptions :(
join needs to probably do stuff with exception_ptr. ok so then there does have to be shared state, fiddlesticks. probably then instead of unique_ptr of std::function you just want a little inner struct with exception pointer and the function as before, and then the context argument to thread creation is a released unique_ptr of that shared_ptr that's then reconstituted by the same manner as above. that then ensures that one ref count is leaked until the thread actually starts and reclaims it.
This should be mostly fixed now? Dropping the thread will just detach it; and joining it now behaves as intended.
577ef6e478
to11d467fecd
looks sound to me
it does lose exceptions (most likely just into exploding tho, which is ok), which is somewhat bothersome, but this is an improvement for now, i think.
WIP: Use our own Thread struct instead of std::threadto Use our own Thread struct instead of std::thread