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 lookupNetwork deadlock #948

Merged
merged 1 commit into from Nov 8, 2022
Merged

Fix lookupNetwork deadlock #948

merged 1 commit into from Nov 8, 2022

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Nov 8, 2022

This fixes a deadlock accidentally introduced as part of #903. That PR introduced an early exit in the net_version callback that did not release the lock.

Checklist

  • Tests are included if applicable
  • Any added code is fully documented

@Gudahtt Gudahtt requested a review from a team as a code owner November 8, 2022 21:05
@Gudahtt
Copy link
Member Author

Gudahtt commented Nov 8, 2022

Unfortunately this part of the controller is not tested, so we'd have to backfill tests somewhat to get this adequately covered. It would also be challenging because we don't have network mocks setup yet for these tests. For this reason I'd like to skip writing tests for this change for now, due to the urgency of getting this released for the next mobile update and ahead of the monorepo branch.

This fixes a deadlock accidentally introduced as part of #903. That PR
introduced an early exit in the `net_version` callback that did not
release the lock.

A `try...finally` block has been introduced to ensures the lock is
always released.
@Gudahtt Gudahtt merged commit b3635c9 into main Nov 8, 2022
@Gudahtt Gudahtt deleted the fix-network-lookup-deadlock branch November 8, 2022 21:11
gantunesr pushed a commit that referenced this pull request Dec 8, 2022
This fixes a deadlock accidentally introduced as part of #903. That PR
introduced an early exit in the `net_version` callback that did not
release the lock.

A `try...finally` block has been introduced to ensures the lock is
always released.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
This fixes a deadlock accidentally introduced as part of #903. That PR
introduced an early exit in the `net_version` callback that did not
release the lock.

A `try...finally` block has been introduced to ensures the lock is
always released.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
This fixes a deadlock accidentally introduced as part of #903. That PR
introduced an early exit in the `net_version` callback that did not
release the lock.

A `try...finally` block has been introduced to ensures the lock is
always released.
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.

None yet

2 participants