Check for chance of class init deadlock with non-nested subclass #4429
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
tkindy
commented
Jun 10, 2024
Comment on lines
-147
to
150
| if (!isStatic(use)) { | ||
| if (use.isEnclosedBy(classSymbol) && !isStatic(use)) { | ||
| // Nested inner classes implicitly take the enclosing instance as a constructor parameter, | ||
| // and can't be initialized without first initializing their containing class. | ||
| return; |
There was a problem hiding this comment.
From the comment, it's clear the intention was to only match situations where the subclass is a non-static nested class of the superclass. But it's overly broad; any class that isn't a static inner class (since only inner classes can be marked static) will be matched by the current conditional. First checking if the class is enclosed by the defining class makes the conditional true to the comment.
|
Thanks for catching this! |
cushon
approved these changes
Jun 10, 2024
copybara-service bot
pushed a commit
that referenced
this pull request
Jun 11, 2024
This PR fixes a bug with the `ClassInitializationDeadlock` check to find chances for a class initialization deadlock when the subclass isn't nested within the superclass. This situation [can still produce a deadlock](https://gist.github.com/tkindy/a83ec86c95bb7d7119f6b208d239ea94). Fixes #4429 FUTURE_COPYBARA_INTEGRATE_REVIEW=#4429 from HubSpot:non-nested-class-init-deadlock c5cc883 PiperOrigin-RevId: 641925847
benkard
pushed a commit
to benkard/jgvariant
that referenced
this pull request
Jul 19, 2024
This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [com.google.errorprone:error_prone_core](https://errorprone.info) ([source](https://github.com/google/error-prone)) | | minor | `2.28.0` -> `2.29.2` | | [com.google.errorprone:error_prone_annotations](https://errorprone.info) ([source](https://github.com/google/error-prone)) | compile | minor | `2.28.0` -> `2.29.2` | --- ### Release Notes <details> <summary>google/error-prone</summary> ### [`v2.29.2`](https://github.com/google/error-prone/releases/tag/v2.29.2): Error Prone 2.29.2 [Compare Source](google/error-prone@v2.29.1...v2.29.2) This release contains all of the changes in [2.29.0](https://github.com/google/error-prone/releases/tag/v2.29.0) and [2.29.1](https://github.com/google/error-prone/releases/tag/v2.29.1), plus: - a bug fix for a crash in `TraditionalSwitchExpression` (google/error-prone#4479) - restores the `module-info` for the annotations jar, which was accidentally removed (google/error-prone#4480) Full Changelog: google/error-prone@v2.29.1...v2.29.2 ### [`v2.29.1`](https://github.com/google/error-prone/releases/tag/v2.29.1): Error Prone 2.29.1 [Compare Source](google/error-prone@v2.29.0...v2.29.1) This release contains all of the changes in [2.29.0](https://github.com/google/error-prone/releases/tag/v2.29.0), plus: - a bug fix to `UnusedVariable` to handle unnamed `_` variables (google/error-prone#4451) - a bug fix for a crash in `SetUnrecognized` (google/error-prone#4475) Full Changelog: google/error-prone@v2.29.0...v2.29.1 ### [`v2.29.0`](https://github.com/google/error-prone/releases/tag/v2.29.0): Error Prone 2.29.0 [Compare Source](google/error-prone@v2.28.0...v2.29.0) New checks: - [`MissingRuntimeRetention`](https://errorprone.info/bugpattern/MissingRuntimeRetention) - [`SetUnrecognized`](https://errorprone.info/bugpattern/SetUnrecognized) - [`StatementSwitchToExpressionSwitch`](https://errorprone.info/bugpattern/StatementSwitchToExpressionSwitch) Closed issues: [#​4318](google/error-prone#4318), [#​4429](google/error-prone#4429), [#​4467](google/error-prone#4467) Full Changelog: google/error-prone@v2.28.0...v2.29.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
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.
This PR fixes a bug with the
ClassInitializationDeadlockcheck to find chances for a class initialization deadlock when the subclass isn't nested within the superclass. This situation can still produce a deadlock.