-
Notifications
You must be signed in to change notification settings - Fork 175
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
Bug fix and some tests #1948
Merged
Merged
Bug fix and some tests #1948
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
galabovaa
commented
Sep 24, 2024
•
edited
Loading
edited
- Fixes deadlock issue on windows
- Updates to HighsTaskExecutor
- Adds sanitizers in CMake
- Linux only so far
- Workflow WIP
- Fixes cmake regex error for githash
Removes synchronization with worker threads on shutdown. Also removes the "search" for the main executor in the worker threads. Instead we simply pass the main executor to the thread as a parameter. We also pass the underlying shared_ptr to avoid potential edge cases where reference count drops to zero before some threads initialize. I made the run_worker static to avoid any confusion about "this" vs "executor->ptr", and so it uses the shared_ptr to reference the shared memory. The last worker thread will delete the shared memory, via the shared_ptr reference count.
…ensure shared memory is kept alive until all threads have finished. Changed identification of main thread to use std::thread::id rather than thread_local memory pointer. Added vector of workerThreads which can be detached or joined on shutdown. Ensured that shutdown can only block if called on main thread, otherwise it might be possible to deadlock. Manually using cache_aligned memory allocation, it was used previously with shared_ptr and I wanted to keep it just in case. Note: I had some weird issues when compiling with /MD flag with mvsc. It would run but often crash. /MT flag works consistently for me.
This was referenced Sep 24, 2024
Closed
jajhall
approved these changes
Sep 25, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no way that I can make any comment on this!
I have to trust you and Leona!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.