Fix flaky use-after-free in udp_server #10015
Conversation
src/core/lib/iomgr/udp_server.c
Outdated
| /* Registered for both read and write callbacks: increment active_ports | ||
| * twice to account for this, and delay free-ing of memory until both | ||
| * on_read and on_write have fired. */ | ||
| s->active_ports = 2; |
There was a problem hiding this comment.
Did you mean to use += instead of =?
There was a problem hiding this comment.
Ah yeah, good point. Done.
|
Oh hang on, not sure that the tests actually ran. I saw a big green tick mark that said "All checks passed", but now after merging it says "1 of 2 checks passed". How do I verify that tests have passed while a pull request is ongoing? I don"t see any comments from Jenkins in the comment thread. |
|
I think the tests will start to run within a short period after a new commit is pushed. Before they start, it says all passed :(. Since this change is limited to udp_server, let"s wait and see if there is anything going wrong on master. We can revert it if something related to this PR goes wrong. |
|
It turns out this PR includes a submodule change, which is breaking the master. I am reverting it. |
|
@yang-g I don"t get it - what did I do that introduced a submodule change? |
|
Somehow in the PR, gflags is changed from 30dbc81fb5ffdc98ea9b14b1918bfe4e8779b26e to f8a0efe03aa69b3336d8e228b37d4ccb17324b88. That caused sanity test to fail on master. |
Previously if both on_read and on_write were armed, and shutdown occurred, then one would fire first, decrement active_ports, and free the underlying udp_server structure. See the (error != GRPC_ERROR_NONE) handling in on_read or on_write.
This fix sets the active_ports ref count to 2, so that both on_read and on_write must fire or be cancelled before the memory is freed.