Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@swernli
Copy link
Collaborator

@swernli swernli commented Aug 13, 2020

This fixes an intermittent crash by putting shared locks around the access of the psis vector, where create and destroy use an exclusive lock of the same mutex

Fixes #335

@swernli
Copy link
Collaborator Author

swernli commented Aug 13, 2020

I've introduced a bug somewhere with how the new get method is working. Changing this PR to draft while I track it down.

@swernli swernli marked this pull request as draft August 13, 2020 22:40
This fixes an intermittent crash by putting shared locks around the access of the `psis` vector, where create and destroy use an exclusive lock of the same mutex

Fixes #335
@swernli swernli force-pushed the swernli/fix-namespaces branch from 75ce934 to 83e0c6e Compare August 13, 2020 22:53
@swernli swernli marked this pull request as ready for review August 14, 2020 01:35
{
namespace Simulator
{
mutex_type _mutex;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dbwz8 This code used to use the #define that resolves to omp_mutex when OpenMP support is enabled. However, this code doesn't interact with any other OpenMP code/classes, so I think it should be safe to use the std class instead, especially since we compile with C++17 across all platforms now. Please let me know if this assumption is safe, and if not we can look at other solutions!

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks OK to me.

Copy link
Contributor

@bettinaheim bettinaheim left a comment

Choose a reason for hiding this comment

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

lgtm, but Dave is more familiar with the simulator than I am.

@swernli swernli merged commit 5de1e41 into master Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simulator tests intermittently fail with Access Violation crashes

4 participants