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

[7.4.0] Fix an NPE with the SkymeldUiStateTracker. #23495

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

joeleba
Copy link
Member

@joeleba joeleba commented Sep 3, 2024

A race condition can happen:

  • Thread 1: #writeProgressBar is called due to LoadingPhaseStartedEvent, expecting buildStatus to be TARGET_PATTERN_PARSING
  • Main thread: LoadingPhaseCompleteEvent arrives. buildStatus is set to LOADING_COMPLETE. additionalMessage is not yet updated and is still null.
  • Thread 1: Seeing that buildStatus is now LOADING_COMPLETE, it moves forward with that code path => NPE when additionMessage is checked.

To fix this NPE, the CL made 2 changes:

  1. Synchronize methods that touch buildStatus
  2. Default additionalMessage to an empty string to make the code more robust.

PiperOrigin-RevId: 640061798
Change-Id: I5034fe7692e0641559fb7666eec09cb554e09545

#23484

A race condition can happen:

- Thread 1: `#writeProgressBar` is called due to `LoadingPhaseStartedEvent`, expecting `buildStatus` to be `TARGET_PATTERN_PARSING`
- Main thread: `LoadingPhaseCompleteEvent` arrives. **`buildStatus` is set to `LOADING_COMPLETE`**. additionalMessage is not yet updated and is still null.
- Thread 1: Seeing that `buildStatus` is now `LOADING_COMPLETE`, it moves forward with that code path => NPE when `additionMessage` is checked.

To fix this NPE, the CL made 2 changes:

1. Synchronize methods that touch `buildStatus`
2. Default `additionalMessage` to an empty string to make the code more robust.

PiperOrigin-RevId: 640061798
Change-Id: I5034fe7692e0641559fb7666eec09cb554e09545
@joeleba joeleba requested a review from a team as a code owner September 3, 2024 08:47
@joeleba joeleba requested review from meisterT and removed request for a team September 3, 2024 08:47
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-CLI Console UI labels Sep 3, 2024
@iancha1992 iancha1992 added this pull request to the merge queue Sep 3, 2024
@iancha1992 iancha1992 changed the title Fix an NPE with the SkymeldUiStateTracker. [7.4.0] Fix an NPE with the SkymeldUiStateTracker. Sep 3, 2024
Merged via the queue into bazelbuild:release-7.4.0 with commit 77ed5ee Sep 3, 2024
51 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 3, 2024
@joeleba joeleba deleted the npe-fix branch September 17, 2024 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-CLI Console UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants