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

Complete channel initialization in the event loop #7509

Closed
wants to merge 1 commit into from

Conversation

ulfjack
Copy link
Contributor

@ulfjack ulfjack commented Feb 22, 2019

If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last non-pending handler. We therefore
have to make sure to involve the event loop before marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes #7464.

If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last _non-pending_ handler. We therefore
have to make sure to involve the event loop _before_ marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes bazelbuild#7464.
@meteorcloudy
Copy link
Member

Maybe this PR also fixes #7505?

@ulfjack
Copy link
Contributor Author

ulfjack commented Feb 22, 2019

I ran it 15 times with my patch and I didn't see a hang.

@meteorcloudy
Copy link
Member

meteorcloudy commented Feb 22, 2019

Cool, then please also add "fixes #7505" in the description.

@bazel-io bazel-io closed this in f9eb1b5 Feb 22, 2019
laurentlb pushed a commit that referenced this pull request Feb 22, 2019
If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last _non-pending_ handler. We therefore
have to make sure to involve the event loop _before_ marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes #7464.

Closes #7509.

GERRIT_CHANGE_ID=
PiperOrigin-RevId: 235184010
@buchgr
Copy link
Contributor

buchgr commented Feb 25, 2019

Great catch @ulfjack and @Reflexe! Netty's channel pool seems to pick a random [1] event loop at initialization where it runs all channel pool tasks on. In the case of the HttpBlobStore this will be true 50% of the time (we have 2 event loops). In this code I was wrongly assuming that the returned Future always runs its listeners on the eventloop of the acquired channel.

[1] https://github.com/netty/netty/blob/4.1/transport/src/main/java/io/netty/channel/pool/FixedChannelPool.java#L228

laurentlb pushed a commit that referenced this pull request Feb 28, 2019
If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last _non-pending_ handler. We therefore
have to make sure to involve the event loop _before_ marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes #7464.

Closes #7509.

GERRIT_CHANGE_ID=
PiperOrigin-RevId: 235184010
laurentlb pushed a commit that referenced this pull request Mar 7, 2019
If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last _non-pending_ handler. We therefore
have to make sure to involve the event loop _before_ marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes #7464.

Closes #7509.

GERRIT_CHANGE_ID=
PiperOrigin-RevId: 235184010
laurentlb pushed a commit that referenced this pull request Mar 7, 2019
If addLast is called outside an event loop, then the handler is added in
the 'pending' state. Sending an event to the pipeline does not send it
to the last handler, but to the last _non-pending_ handler. We therefore
have to make sure to involve the event loop _before_ marking the channel
as ready to be used.

Thanks to @Reflexe who pointed me in the right direction.

Fixes #7464.

Closes #7509.

GERRIT_CHANGE_ID=
PiperOrigin-RevId: 235184010
@ulfjack ulfjack deleted the http-blob-store-race branch December 7, 2019 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants