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 race condition in network controller lookup() method. #5892

Merged
merged 1 commit into from
Dec 7, 2018

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Dec 7, 2018

This PR fixes a bug in the network controller that can be reproduced as follows:

  • on the settings page, add a network that doesn't exist
  • select ropsten (or any other infura supported network)
  • select the custom network you just added and then immediately select ropsten again
  • after a few seconds, infinite spinner will be shown and it will only be removable by switching networks and switching back

The bug is caused by something of a race condition in the network controller. When lookup() is called in controllers/network/network.js, as happens after a new network is selected, a net_version rpc call is made via ethQuery.sendAsync. If a custom rpc network which does not support the net_version is selected, the request for it will fail. The networkMiddleware created by createJsonRpcClient.js will be retried in eth-json-rpc-middleware/fetch 5 times, with a 1 second delay between each try. It will pass an error to the callback after the 5th try. ethQuery.sendAsync({ method: 'net_version' } in controllers/network/network.js handles that error by setting the network state to "loading".

If a network was successfully selected while those 5 retries were in process, the network state will incorrectly be set to 'loading', even though a network is loaded.

This PR corrects this by checking that the network state before the ethQuery.sendAsync({ method: 'net_version' } call is the same as right after the callback resolves. It does not update the network if it has changed while waiting for the callback to be called.

This is a relatively simple fix for the issue. Ideally, we would have a way of cancelling a net_version request that is already in process, but larger architectural changes will be needed.

@tmashuang tmashuang merged commit 575fb60 into develop Dec 7, 2018
@tmashuang tmashuang deleted the fix-net-version-race-condition branch December 7, 2018 22:49
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