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

MC-SAT Thread Safety #456

Merged
merged 2 commits into from
Sep 22, 2023
Merged

Conversation

markpmitchell
Copy link
Contributor

This pull request contains two components (each in its own commit):

  1. Someone had tried to add debug code to detect deadlocks where a thread requests a lock that it already has. However, (a) that code was incorrectly conditionalized, meaning that it was never used, even with YICES_MODE=debug, and (b) the code was incorrect because it failed to initialize the POSIX mutex attribute. Fixed.
  2. Running the test suite with YICES_MODE=debug with YICES_MODE=debug revealed a number of places where the MC-SAT code was calling yices_* API functions (which take the global lock) rather than o_yices* functions (which do not). Since the MC-SAT code is only reachable via the yices_* API, the global lock is already held at that point; there is no reason to take it again.

With these changes, the test suite passes using --enable-mcsat --enable-thread-safety. Of course, the testsuite doesn't exercise the multi-threaded mode and we have no way to prove that MC-SAT is 100% thread-safe. But, this would seem to be a step forward.

@coveralls
Copy link

Coverage Status

coverage: 64.973% (-0.01%) from 64.983% when pulling 74f5f3b on markpmitchell:mcsat-thread-safety into 6903281 on SRI-CSL:master.

if(retcode){
fprintf(stderr, "create_yices_lock failed: pthread_mutexattr_settype returned %d\n", retcode);
}
#ifndef NDEBUG
Copy link
Member

Choose a reason for hiding this comment

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

minor style thing:
can we #ifdef DEBUG?

@markpmitchell
Copy link
Contributor Author

markpmitchell commented Sep 22, 2023 via email

@ahmed-irfan
Copy link
Member

I understand.

Copy link
Member

@ahmed-irfan ahmed-irfan left a comment

Choose a reason for hiding this comment

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

LGTM

@ahmed-irfan ahmed-irfan merged commit 5d2aafe into SRI-CSL:master Sep 22, 2023
9 checks passed
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.

3 participants