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 potential IndexOutOfBoundsException in curActorIsBlocking and concurrentActorCausesBlocking #277

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

eupp
Copy link
Collaborator

@eupp eupp commented Feb 20, 2024

both properties access the current actor id actorId = currentActorId[iThread] but do not handle the case when actorId = -1, that is the case before any actor has started executing

…currentActorCausesBlocking

* both functions get the current actor id `actorId = currentActorId[iThread]` but do not handle the case when `actorId = -1`,
  that is the case before any actor has started executing

Signed-off-by: Evgeniy Moiseenko <[email protected]>
@ndkoval
Copy link
Collaborator

ndkoval commented Feb 20, 2024

Could you please add a test?

@eupp
Copy link
Collaborator Author

eupp commented Feb 21, 2024

@ndkoval I've looked into this, and it seems it would be hard to trigger the error and make such test deterministic.
For this problem to occur, it has to be the case, that one thread has started working and detected deadlock, while other threads has not yet started execution (i.e. did not call onActorStart for the first time). We have no control over threads execution order on this level.

One possible way to write test would be to manually create ModelCheckingStrategy instance and call its methods in specific order to trigger the error. Do you think we should take this path?

@ndkoval
Copy link
Collaborator

ndkoval commented Feb 21, 2024

@eupp, in this case, I would suggest adding a comment describing why the added logic is required and leaving a link to this PR.

@ndkoval
Copy link
Collaborator

ndkoval commented Feb 21, 2024

Maybe it would be better to rewrite the code in a way similar to

// <comment describing the issue>
if (actorId < 0) return null

thus, covering this weird path separately

@ndkoval ndkoval self-requested a review February 21, 2024 20:03
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Copy link
Collaborator

@ndkoval ndkoval left a comment

Choose a reason for hiding this comment

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

Please fix the build failure on Java 17 (Mac).

@ndkoval ndkoval merged commit c33be44 into develop Feb 27, 2024
12 of 14 checks passed
@ndkoval ndkoval deleted the fix-curActorIsBlocking branch February 27, 2024 11:07
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.

2 participants