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

Prevent some compiler optimizations #136

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

accelerated
Copy link
Contributor

@accelerated accelerated commented Nov 15, 2020

Describe your changes

  • Fix compiler optimizations and minor bugs caused by using -03
  • Updated travis to build with RelWithDebInfo instead of Debug

Notable bugs found

  • On quantum_spinlock_util_impl.h:329 in spinWaitWriter() the closing parenthesis was in the wrong place.
  • In lockRead, lockWrite and upgradeToWriteImpl the check for spinWaitReader and spinWaitWriter was checking against != Attempt::Once instead of ==Attempt::Unlimited. When upgrading coroutines to write, the attempt is set to Attempt::Reentrant which was causing the coroutine to enter this spinwait loop. Since this spin wait loop does not yield the coroutine, the first coroutine being upgraded would take the lock, waiting for all other coroutine readers to either upgrade or release their read locks. However since all other coroutines could never run (because of the non-yield) this would result in a deadlock. Previous upgradeToWrite tests only involved scenarios with threads which did not have this problem. A different test using coroutines is now added to check proper behavior in both cases.

Signed-off-by: Alexander Damian adamian@bloomberg.net

Testing performed

  • Added more tests for coroutine synchronization for ReadWriteLocks.

- Fix compiler optimizations and minor bugs caused by using -03
- Updated travis to build with RelWithDebInfo instead of Debug

Signed-off-by: Alexander Damian <adamian@bloomberg.net>
Copy link
Contributor

@arosenzweig3 arosenzweig3 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@accelerated accelerated merged commit 0f58d39 into bloomberg:master Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants