Skip to content
This repository was archived by the owner on Nov 9, 2023. It is now read-only.

fix: race issue in stream processing#47

Merged
Gudahtt merged 2 commits intoMetaMask:mainfrom
zzmp:fix-race
Jun 14, 2023
Merged

fix: race issue in stream processing#47
Gudahtt merged 2 commits intoMetaMask:mainfrom
zzmp:fix-race

Conversation

@zzmp
Copy link
Copy Markdown
Contributor

@zzmp zzmp commented Jun 12, 2023

Fixes Uniswap/interface#6613 by resolving a race condition with ReadableStream pipes: initialized pipes may process data synchronously, but uninitialized pipes will process data asynchronously (only after being initialized). This can manifest in any dApp that MetaMask runs on as a hanging stream, and unresponsive MetaMask provider.

  • Adds a test exposing the race condition
  • Reorders setting state and sending to stream to avoid the race condition

@zzmp zzmp requested a review from a team as a code owner June 12, 2023 23:54
Gudahtt
Gudahtt previously approved these changes Jun 14, 2023
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM! Great catch. I wasn't able to understand the linked issue in detail, but regardless this seems like a sensible change.

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.

sometimes the connection doesn't work

2 participants