fix(sock): Use the updated cancellation cb interface. #1940
Conversation
src/facade/dragonfly_connection.cc
Outdated
| } | ||
|
|
||
| ConnectionFlow(peer); | ||
|
|
||
| if (break_poll_id_ != UINT32_MAX) { | ||
| socket_->CancelPoll(break_poll_id_); | ||
| if (breaker_cb_) { |
There was a problem hiding this comment.
if you do this, you must make sure CancelOnErrorCb() can be called multiple times. we had some issues with shutting down a connection and data race around this codepath. you can see I used to reset break_poll_id_ to avoid calling CancelPoll if OnBreakCb was called first.
There was a problem hiding this comment.
Also, we must ensure that OnBreakCb won't be called once cc_.reset(); is called. Currently, we have such cases but I am not sure why
There was a problem hiding this comment.
About the race with cc_.reset(); - how does it work with io_uring?
royjacobson
force-pushed
the
cancellation_cb_epoll
branch
4 times, most recently
from
94eb02b to
8de145c
Compare
|
This seems to still fail: But it just silently times out and I'm not sure if it's my fault or not. |
|
@royjacobson I see the regression tests finish with 3 minutes in other CIs and here we get 48 minutes. Does it pass on your laptop? |
Tested locally, |
|
@royjacobson Thank you, I will take it from here. Best wishes |
|
Will add romange/helio#155 here once its merged to improve error diagnostics |
|
Hi @dranikpg is this ready? are you still waiting on helio? |
Signed-off-by: Vladislav Oleshko <[email protected]>
dranikpg
force-pushed
the
cancellation_cb_epoll
branch
from
9b9a076 to
d23e1f7
Compare
|
Sure, I forgot to merge in helio then 😅 I've rebased just now on main which should contain the latest helio pull |
Fixes #1939