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

Multiple suspension points support #348

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

avpotapov00
Copy link
Collaborator

@avpotapov00 avpotapov00 commented Jul 28, 2024

Closes #122

@avpotapov00 avpotapov00 requested a review from ndkoval July 28, 2024 22:56
@avpotapov00 avpotapov00 self-assigned this Jul 28, 2024
@eupp eupp self-requested a review August 16, 2024 10:48
@eupp
Copy link
Collaborator

eupp commented Aug 16, 2024

We discussed this PR today with @avpotapov00 , and come to the conclusion that it overlaps with #361.

This PR implements three things:

  1. Fix for events tracking in a resumption part of a coroutine.
  2. Fix for the trace collection in a resumption part of a coroutine.
  3. Multiple suspension points support.

Independently, while working on the new model checking strategy, I also discovered problems (1) and (2), and opened PR #361.

At first I was only aware that this PR fixes (1), so I borrowed the relevant code in the commit ab760ef.

Then I independently developed different fix for (2).
As we discussed today, this fix also handles more complicated case, when the resumed part of the coroutine has several nested suspend functions. To handle this case, I save the stack of suspended functions, and then restore it on resumption.

So after the discussion, we thought that it would be good idea to split this PR into two parts.

  1. The first part should contain fixes for problems (1) and (2), and will be delivered under Fix resumption trace collection and printing logic #361
  2. The second part should contain the multiple suspension points logic, and should be delivered after the first part.

@eupp eupp force-pushed the 347-support-multiple-suspension-points branch from 3c70b8f to 83a7d5d Compare December 26, 2024 17:30
@eupp eupp force-pushed the 347-support-multiple-suspension-points branch from 83a7d5d to 13ffe5c Compare December 26, 2024 17:31
eupp added 2 commits December 26, 2024 20:30
Signed-off-by: Evgeniy Moiseenko <[email protected]>
Signed-off-by: Evgeniy Moiseenko <[email protected]>
@eupp
Copy link
Collaborator

eupp commented Dec 26, 2024

@avpotapov00 @ndkoval I've rebased the PR on develop, please have a look.

val (resumedValue, continuation) = suspendResultToContinuation
// Erase the current result
completion.resWithCont.set(null)
// We must exit the ignored section to keep tracking execution after the resumption.
runOutsideIgnoredSection(thread) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should this code be called outside the ignored section?

}
if (guarantee == ManagedGuaranteeType.TREAT_AS_ATOMIC) {
if (guarantee == ManagedGuaranteeType.TREAT_AS_ATOMIC &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please add a comment explaining what is going on here?

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