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

Ivar.fill crash fix: Remove outstanding_requests ivars during libp2p_helper shutdown/restart #7575

Merged
merged 1 commit into from Jan 21, 2021

Conversation

mrmr1993
Copy link
Member

This PR removes the Ivar.ts from outstanding_requests in Mina_net2 when a shutdown is detected.

This avoids a double-fill when

  • we perform a scheduled restart of libp2p_helper (or it crashes and we restart it) in Gossip_net.Libp2p.create
  • there is some message read from stdout of libp2p_helper, and the scheduler yields before handling it
  • the termination handler in Mina_net2 fills all unfilled Ivar.ts with an error state
  • the scheduler resumes the message handling, which attempts to fill the now-filled Ivar.t.

A backtrace indicating this logic (or, at least, the final step) was posted on discord in the development channel.

Checklist:

  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them:

@mrmr1993 mrmr1993 requested a review from a team as a code owner January 21, 2021 00:05
@mrmr1993 mrmr1993 added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Jan 21, 2021
@mrmr1993 mrmr1993 added this to In Review in MainNet Jan 21, 2021
@mrmr1993 mrmr1993 merged commit fa8b9ea into compatible Jan 21, 2021
@mrmr1993 mrmr1993 deleted the fix/mina_net2_ivar_fill branch January 21, 2021 01:56
@aneesharaines aneesharaines moved this from In Review to Done in MainNet Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants