Modify threadpool teardown #218
Merged
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
…t thread than locked it.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #218 +/- ##
===========================================
- Coverage 75.20% 75.19% -0.01%
===========================================
Files 52 52
Lines 10182 10188 +6
===========================================
+ Hits 7657 7661 +4
- Misses 2525 2527 +2
☔ View full report in Codecov by Sentry. |
|
undefined-behavior-test failed due to race condition within test fixture, no issue with actual module being tested. |
niyatim23
reviewed
Sep 18, 2023
niyatim23
approved these changes
Sep 19, 2023
disa6302
approved these changes
Sep 19, 2023
hassanctech
added a commit
that referenced
this pull request
Mar 11, 2024
* Fix definition of LP64 for gcc * Use INT64 instead of LL so the type is will consistent across platforms * Modify threadpool teardown (#218) * Modify threadpool teardown to avoid unlocking mutexes from a different thread than locked it. * Remove an extra comma that made the format incorrect (#216) * Use const PCHAR so C++ compilers won't complain about passing strings constants (#215) * Add stream state machine iterator (#220) * Define iterator function, add a Stream member var * Replace recursive stepState calls with iterator * Replace all Stream stepState calls with iterator * Fix existing comment typos * CLANG * Rename 'keepIterating' member to be more clear * Add null pointer checks to iterator * CLANG * Stylistic change based on precedent * Remove redundant pStateMachine null check * Remove another redundant null pointer check * Fix null checks * Add timerqueuekick and checkstatemachinetransition (#221) * Add timerqueuekick and checkstatemachinetransition * Increase sleep at end of threadpool test to give spawned threads time to free their memory * Utils - Remove duplicate error code * m1 build * update cmake * fix incompatible pointer error * fix incompatible pointer error * run tests * cmake once * no typecasting * Revert "no typecasting" This reverts commit f2096b8. * Revert "Revert "no typecasting"" This reverts commit 7d53af0. * everything working * Add gcc-M1 CI test (#225) * Add gcc M1 test * testing * more * Update gcc path * Try using LLP64 for Apple * Add is null or empty * Update CommonDefs.h (#229) * Fix 32 bit crash issue (#230) * fix 32-bit crash in threadpool and possible seg fault if get item was null * clang format * remove null item and keep iterating * properly free threadpool if failed to create (#231) * address compiler warnings, cleanup ci (#232) * remove unnecessary double compiles, separate out test ci run into a separate section, address compile warning * address multiple compile warnings * address more compiler warnings for linux, separate out few more tests from build in ci * move stuff around * use consistent flags in pic library and test * clang format * only run test if flag FIXUP_ANNEX_B_TRAILING_NALU_ZERO is set, previously we re-built pic with this flag for the tests but the test itself segfaults if it's not set * put tests behind ifdefs which require flags set * tweak ci yml * more ifdefing tests * more ifdefs for tests * more changes * correct path for tsan/ubsan suppressions files * address warning for linux implicit decl of pthread_getname_np * define only once * more ifdefs * more ifdefs for tests * ifdef more tests * more ifdefs * more ifdefs * more ifdefs * more ifdefs * more ifdefs * if thread count is going to overflow, return 0 * another if/def * add build and testing with ALIGNED_MEMORY_MODEL and FIXUP_ANNEX_B_TRAILING_NALU_ZERO to test both paths in CI * more ifdefs for unalign access * more ifdefs for ubsan * ifdef more tests for aligned memory model * more memory aligned ifdefs for tests * more if defs in tests * make sure correct number of bytes for snprintf * State machine name for logs (#233) * Add state machine tag * Add tag for client state machine * Use char with fixed size * Use default tags * Random string * Allow tag setting for state machines * Add unit tests * Free state machine * Use createStateMachineWithTag instead * Rename * Address comments * Silent (#234) * Remove const in the api definition for getStateMachineName (#235) * char* * Remove const * Enable tsan (#236) * enable tsan * lock access to global count to address tsan violation * remove globals, use struct to thread * some changes * lock to protect data race in case of canceled thread still * lock around terminate in tests for tsan data race * free mutex in tests * Release build (#237) * Switch to ASSERT_EQ from assert to prevent release build optimization * Fix assert * Cleanup log statements * Threadpool tsan (#238) * Protect accessing threadpool with threaddata lock * Fix potential seg fault from attempting to access queue after threadpool has been destroyed by locking the dataMutex around that queue, and having the destructor pass waiting tasks onto the queue when threads are stuck waiting for on said queue * Fix typo from merge * Reduce sleeps in threadpool * Lock test utils mutex, and check for termination in actors before assigning pQueue * need allocators set check * wait on teardown of test threads * removing accessing gTerminateCount in the for loop * longer sleep to wait for threads to free their heap * dramatically increase sleep * Add sleep in the heap check in case it's still waiting for a thread to exit * Infinite loop if memory isn't clear. Tests will timeout on GHA if the memory leak actually exists. If it's just waiting for threads to clean up, then it will keep waiting. * Remove sleep from termination task * Assert handle (#239) * Use abort instead * custom assert * Fix definition * Fix fprintf * Simple assert * Clang formatted * Fix build issue * safe gmtime, guard with mutex since not thread safe on non windows pl… (#240) * safe gmtime, guard with mutex since not thread safe on non windows platforms * clang format * add cygwin to keep original gmtime since pthread isn't used there in our thread impl * address pr comments, fix compile warnings in threadpool * missed the extern variable name change * add EXPECT_EQ around thread create and thread join calls in the unit tests, possibly address tsan complain where it it looks like multiple tests executing at the same time * unlock stream mutex before acquiring the fom mutex to avoid deadlock with putframe (#242) * Update ci.yml using xcode 14.3.1 results in a linker issue with gcc, 15.1 fixes the issue, trying 15.2 since this appears to be the latest available. * Update ci.yml ci.yml formatting * Reset current index in case it overflows out of range (#245) * Add PR description lint (#248) * Merge pull request #249 from awslabs/fix-read-windows Fix readFile on Windows --------- Co-authored-by: Dave Johansen <[email protected]> Co-authored-by: jdelapla <[email protected]> Co-authored-by: Stefan Kieszkowski <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: Niyati Maheshwari <[email protected]> Co-authored-by: Niyati Maheshwari <[email protected]> Co-authored-by: Jeremy Gunawan <[email protected]> Co-authored-by: Divya Sampath Kumar <[email protected]>
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.
Threadpool teardown has been changed so that the threads owned by the threadpool will not need to unlock the ThreadData mutex on teardown. This was accomplished by using the terminate atomic bool in place of the locked mutex to indicate that the threadpool has been destroyed.