client_channel: fix crash when cancelling a watch after SHUTDOWN #30885
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markdroth
added
the
release notes: no
grpc-checks
bot
added
per-call-memory/neutral
per-channel-memory/neutral
bloat/low
labels
veblush
approved these changes
Sep 9, 2022
markdroth
added a commit
to markdroth/grpc
that referenced
this pull request
Sep 9, 2022
markdroth
added a commit
that referenced
this pull request
Sep 9, 2022
copybara-service
bot
added
the
imported
veblush
added a commit
that referenced
this pull request
Sep 20, 2022
* Bump 1.49.x branch to 1.49.0.pre1 (#30615) * bump version to 1.49.0-pre1 * regenerate projects * [backport][v1.49.x] forkable fixes (#30646) * Fix forkable globals (#30608) * Fix forkable repeated registration (#30642) This fixes a bug that could occur on repeated grpc initialization (after a complete shutdown) Fixes #30640 * Drop support for ruby 2.5 (#30699) (#30762) * Drop ruby 2.5 support * Backport: "stabilize the C2P resolver URI scheme" to v1.49.x (#30654) * stabilize the C2P resolver URI scheme * Bump 1.49 branch to 1.49.0.pre2 (#30786) * bump version to 1.49.0-pre2 * regenerate projects * Update protobuf on ancillary packages (#30795) (#30805) * Bump release version on 1.49 to 1.49.0.pre3 (#30814) * bump version to 1.49.0-pre3 * regenerate projects * xDS interop: enable pod log collection in the buildscripts (#30735) (#30856) - Enables pod log collection in all PSM interop jobs implemented in #30594. - Associate test suite runs with their own log file, so it's displayed on "Target Log" tab * xDS interop: buildscripts: fix run_test return status (#30768) (#30875) To capture the return status of the test in run_test the last command must be the call to the test itself. This removes `set +x`, which makes the run_test always return success, and not propagate the test status. I can't find it, but this exact error bit us before. Looks like it leaked to other scripts. The good thing is if the test was executed, it's failure would still be picked up from the result xml. However, if the test framework didn't start in the first place, the result will be false positive. Example: https://source.cloud.google.com/results/invocations/98d3e679-ec8a-40bd-9f36-88179747b0d6/targets ``` /home/kbuilder/.pyenv/versions/k8s_xds_test_runner/bin/python3: Error while finding module specification for 'tests.authz_test' (ModuleNotFoundError: No module named 'tests') + set +x Failed test suites: 0 [ID: 3548168] Command finished after 625 secs, exit value: 0 ``` * client_channel: fix crash when cancelling a watch after SHUTDOWN (#30885) (#30928) * Support Python 3.11 (#30818) (#30944) * Support Python 3.11 * Update build images for 3.11 * Whoopsie * The architecture of this thing is garbage * Silence ownership warning * Account for change in git behavior * Fix directory * I am in great pain * Update Windows and arm linux * Agh * Clean up * Bump 1.49 branch to 1.49.0 (#30974) * bump version to 1.49.0 * regenerate projects * Update protobuf to v21.6 on 1.49.x (#31028) * Update third_party/protobuf * run tools/distrib/python/make_grpcio_tools.py * update build_handwritten.yaml * regenerate projects * Bump v1.49.x to v1.49.1 (#31037) * bump version to 1.49.1 * regenerate projects * Automated change: Fix sanity tests Co-authored-by: gnossen <gnossen@users.noreply.github.com> * Fix ruby windows ucrt build (#31053) Co-authored-by: apolcyn <apolcyn@google.com> Co-authored-by: AJ Heller <hork@google.com> Co-authored-by: Richard Belleville <rbellevi@google.com> Co-authored-by: Sergii Tkachenko <sergiitk@google.com> Co-authored-by: Mark D. Roth <roth@google.com> Co-authored-by: Richard Belleville <gnossen@gmail.com> Co-authored-by: gnossen <gnossen@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Possible fix for b/243689872.
What I think is happening here is that there is already a notification enqueued in the
WorkSerializerfrom here, but before it runs, the state tracker gets a SHUTDOWN notification, which causes it to clear the watcher map, thus destroying the external watcher before the callback runs. The fix is to have the callback hold its own ref to the object.