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: a race condition in Host.resetAllServices #182

Merged
merged 1 commit into from
Jul 28, 2023

Conversation

Hexta
Copy link
Contributor

@Hexta Hexta commented Apr 9, 2023

Fix a race condition in Host.resetAllServices() caused by calling channel.close() as a callback of grpc.Client.waitForReady().

The root cause is gRPC subchannel pool state could become into invalid state when global subchannel pool state is used, and there are multiple channels used the same subchannel from the pool.

Closing the channel inside of waitForReady callback prevent all the remaining callbacks for the subchannel from the correct state transition handling.

Closing the channel inside of waitForReady callback prevents the subchannel's remaining callbacks from the correct state transition handling.

The fix is to use setImmediate to allow the remaining callbacks for the subchannel to process the state transition.

The issue appears with grpc-js v1.6.7.

Fix a race condition in Host.resetAllServices() caused by calling
channel.close() as a callback of grpc.Client.waitForReady().

The root cause is gRPC subchannel pool state could become into
invalid state when global subchannel pool state is used, and there are
multiple channels used the same subchannel from the pool.

Closing the channel inside of waitForReady callback prevent all the
remaining callbacks for the subchannel from the correct state transition
handling.

Closing the channel inside of waitForReady callback prevents
the subchannel's remaining callbacks from the correct state transition handling.

The fix is to use setImmediate to allow the remaining callbacks for the
subchannel to process the state transition.
@Hexta
Copy link
Contributor Author

Hexta commented Apr 10, 2023

Hey @connor4312 could you take a look please?

@connor4312 connor4312 merged commit eb99050 into microsoft:master Jul 28, 2023
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