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

Fix more nagging bugs #1035

Merged
merged 3 commits into from
Nov 10, 2017
Merged

Fix more nagging bugs #1035

merged 3 commits into from
Nov 10, 2017

Conversation

gittyupagain
Copy link
Contributor

This PR addresses a few remaining known issues observed in the tests when run repeatedly.

In particular, there were some timing issues with setting the thread priority that could cause the expected thread priority to be lost if set at just the wrong time.

Another issue, that was introduced in #1033, was causing calls to CoreContext::SignalShutdown to potentially wait for a CoreThread to shutdown, even if the wait flag was not set. The solution was to only wait to Rundown (called from the CoreThread's thread) only after the CoreThread::OnStop function is called if we are in the midst of a shutdown.

Finally, some tests were modified to clean up after themselves. Many times there were no waiting on the contexts, causing them to interfere with the next test.

@gittyupagain gittyupagain added this to the v1.0.5 milestone Nov 9, 2017
@gittyupagain gittyupagain requested a review from jdonald November 9, 2017 23:43
Copy link
Contributor

@jdonald jdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CoreThreadTest.CanElevateAnyPriority and SubContextHoldsParentContext seem to be able to run forever on various platforms now.

@jdonald jdonald merged commit c7a8d6b into master Nov 10, 2017
@jdonald jdonald deleted the fix-more-issues branch November 10, 2017 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants