Add SemaphoreSlim Support for Popup CloseAsync to Avoid Race Conditions#3235
Merged
TheCodeTraveler merged 28 commits intoJun 9, 2026
Merged
Conversation
…ry/finally block
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors popup navigation coordination by moving the SemaphoreSlim(1,1) synchronization from PopupExtensions into PopupPage, adding PopupPage.ShowAsync(...) overloads, and extending serialized navigation to include CloseAsync flows. It also expands unit test coverage across popup primitives, options/defaults, the popup service, and navigation edge-cases.
Changes:
- Added
PopupPage.ShowAsync(...)overloads and centralized navigation serialization via a sharedSemaphoreSlim. - Updated
PopupPage.CloseAsyncto wait for MAUI navigation signals (Window.ModalPopped/Page.NavigatedFrom) before raising popup-closed events. - Added/expanded unit tests across popup APIs (results/options/defaults/service/extensions) and added timeouts to reduce hangs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs | Centralizes semaphore-based navigation serialization in PopupPage and changes CloseAsync to await MAUI navigation confirmation signals. |
| src/CommunityToolkit.Maui/Extensions/PopupExtensions.shared.cs | Simplifies extension methods to delegate navigation work to PopupPage.ShowAsync(...) (removes local semaphore). |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs | Adds tests validating exception message guidance for popup exceptions. |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupResultTests.cs | Adds tests for PopupResult<T> behaviors and interface conformance. |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs | Adds timeouts and new navigation-edge-case tests (custom navigation containers, query attribute forwarding, blocked scenarios). |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupOptionsTests.cs | Adds tests for PopupOptions.Empty defaults and IPopupOptions contract adherence. |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/DefaultPopupSettingsTests.cs | Adds tests verifying defaults and override behavior for popup settings. |
| src/CommunityToolkit.Maui.UnitTests/Views/Popup/DefaultPopupOptionsSettingsTests.cs | Adds tests verifying defaults and override behavior for popup options settings. |
| src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs | Expands test coverage for Shell-based popup service APIs and cancellation/null-argument behaviors. |
| src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs | Adds argument validation tests and a typed default-result test for closing without a typed result. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…ttps://github.com/CommunityToolkit/Maui into Ensure-Popup-Has-Closed-Before-`CloseAsync`-Returns
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
VladislavAntonyuk
approved these changes
Jun 7, 2026
TheCodeTraveler
deleted the
Ensure-Popup-Has-Closed-Before-`CloseAsync`-Returns
branch
June 9, 2026 00:05
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description of Change
This PR builds on the work started in #3234, expanding the scope of the
SemaphoreSlim(1,1)to include bothShowAsyncandCloseAsync.I updated
PopupPage, adding the methodPopupPage.ShowAsyncand moved theSemaphoreSlim(1,1)fromPopupExtensionsintoPopupPage. This simplifies the logic inPopupExtensionsand ensures thatPopupPagehandles the logic for both showing + closing Popups.This PR also updates
PopupPage.CloseAsyncto subscribe to theWindow.ModalPoppedevent andPage.NavigatedFromevent;PopupPage.CloseAsyncnow awaits the result of these MAUI Navigation events before invokingPopupClosed, guaranteeing we only invoke the eventsPopupPage.PopupClosedandPopup.Closedafter MAUI has removed the page from the underlying ModalStack.PR Checklist
approved(bug) orChampioned(feature/proposal)mainat time of PRAdditional information
The existing Unit Tests covering
CloseAsyncall still pass indicating no breaking changes.I also asked CoPilot to have unit tests covering every branch in the following classes which is why there are a few new unit tests added:
PopupExtensionsPopupPopupResultPopupOptionsIPopupOptionsDefaultPopupOptionsSettingsDefaultPopupSettingsPopupServiceIPopupResultPopupPage