Repository navigation
Fix tab navigation event cleanup and avoid synchronous callback reentry - #142
Open
yueyangcode wants to merge 1 commit into
Open
yueyangcode wants to merge 1 commit into
yueyangcode wants to merge 1 commit into
Conversation
saillill
added a commit
to saillill/ExplorerTabUtility-WinUI3
that referenced
this pull request
Oct 13, 2026
…-resume stall Continues the review of w4po/ExplorerTabUtility's open pull requests. Three of them describe defects that are genuinely present in this rewrite as well; this ports the fixes, and records the two that turned out not to apply. w4po#137, losing the window: a folder that is meant to become a tab is hidden first, and then the conversion was started fire-and-forget with an unconditional window.Quit() immediately after. Any failure inside — no new tab within the 2s budget, the window not appearing in the tracking table, Navigate refused, no NavigateComplete2 within 5s — therefore destroyed a window that was already hidden. The tab was never created and the window was gone, so the user's folder simply disappeared. * the conversion's result is awaited, and the window is quit only when it succeeded; * on failure the handler returns in the state a normal, non-converting window ends in, which brings the visibility restore in the finally block into play. That restore is the only thing that undoes the hide, so it was not enough to skip the quit; * the liveness tests now ask how many windows still exist, not how many have ever been tracked. _windowEntryDict.Count answers the latter, and it is not the same question: Explorer can destroy a window without the unhook running, and CabinetWClass handles are recycled, so a dead entry can even compare equal to a live window. Every "are there other windows?" decision read that count, so OpenTabNavigateWithSelection could take its tab-reuse branch against a ghost and drop the navigation; * a power-mode handler rebuilds the shell objects on resume. A modern standby cycle can take the shell down and back without explorer.exe terminating, and possibly without a new process id, in which case neither the process watcher nor the pid check notices and the watcher silently stops managing tabs until it is restarted, while still reporting its hook as active. w4po#142, stale handlers: the temporary NavigateComplete2 subscription was only removed from inside the handler or from the catch, so a navigation that never completed — exactly what the 5s timeout exists for — leaked it for the life of the window's process, along with this method's locals. The removal is now unconditional, the completion source resumes continuations on the pool instead of inline on Explorer's COM thread, and the item selection no longer runs inside a shell event handler. w4po#139 was already fixed here and w4po#140/w4po#131 cover nine languages already, so only the bookmark management idea from the latter is new; it is a 55-file change and is not attempted here. Verified: builds with 0 warnings across all three projects, 165/165 tests pass (7 new ones covering the liveness invariant and the tab-handle mapping that must survive an eviction), and the added Microsoft.Win32.SystemEvents dependency is deployed to both publish folders. Both defects were reported and diagnosed upstream, so the trailers below credit the people who found them. They are the only route into GitHub's contributors list for anyone without commits here, which is what the project page asked for and could not be satisfied by editing the list itself. Co-authored-by: youncyb <33374470+youncyb@users.noreply.github.com> Co-authored-by: yueyangcode <120363362+yueyangcode@users.noreply.github.com>
lazyalex616
added a commit
to lazyalex616/ExplorerTabPlus
that referenced
this pull request
Oct 23, 2026
…cleanup and avoid synchronous callback reentry Conflict in OpenTabNavigateWithSelection with w4po#137 (which made it return Task<bool> and wrapped it in try/catch): adopted w4po#142's NavigationCompletion.WaitAsync flow (deferred SelectItems, guaranteed handler detach) and return its completion result to keep w4po#137's bool contract.
lazyalex616
added a commit
to lazyalex616/ExplorerTabPlus
that referenced
this pull request
Oct 23, 2026
…lection when converting or reusing tabs Conflicts with w4po#137/w4po#142 in ExplorerWatcher: kept w4po#137's awaited conversion result (window is restored if conversion fails) while feeding it w4po#144's CaptureInitialSelectionAsync selection; kept w4po#137's IsFileExplorerWindow guard and bool return on tab reuse and added w4po#144's RestoreSelectionAsync there; replaced w4po#142's one-shot SelectItems after navigation with w4po#144's retrying RestoreSelectionAsync, still invoked after the NavigationCompletion wait (outside the COM callback).
lazyalex616
added a commit
to lazyalex616/ExplorerTabPlus
that referenced
this pull request
Oct 23, 2026
- build-release.yml: drop SignPath jobs; build matrix (net9.0-windows, net481 x x64/x86/arm64) -> zip -> Inno Setup installer -> published (non-draft) release with notes from .github/release-notes/<tag>.md - ci.yml: compile both frameworks on push/PR to main and run the PR w4po#142 navigation regression checks (replaces navigation-regression.yml) - remove publish-chocolatey.yml and publish-winget.yml (they publish under upstream package IDs) - remove FUNDING.yml (upstream author's funding links)
This branch has not been deployed
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 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.
Summary
Related to #104 (persistent loading cursor over File Explorer).
This fixes two concrete lifecycle/reentrancy problems in
OpenTabNavigateWithSelection:NavigateComplete2subscription was never removed when the 5-second event wait timed out. A later navigation could invoke the stale handler and restore an obsolete selection.TaskCompletionSourcealso allowed awaiting code to resume inline on that callback stack.The event handler now only signals a completion source with
RunContinuationsAsynchronously. A small internal helper guarantees unsubscription infinallyafter successful subscription, including navigation exceptions and event timeout. File selection runs only after successful completion, outside the event handler body. Expected COM disconnection during cleanup is tolerated. Foreground activation still occurs immediately after the navigation request, before waiting for the event.Validation
net481Release build succeeded with zero warnings/errors. Local build used .NET 8 SDK and generated COM interop references because full-framework MSBuild was unavailable; those local wrappers/build scripts are not part of this PR.net9.0-windowstarget has not been built locally.Limits / review notes
This is not yet a proven root-cause fix for all instances of #104. Explorer was restarted before the trial, the prior symptom had accumulated over days, and a deterministic before/after reproducer is not available. Longer-term observation and maintainer integration testing are needed, so this PR intentionally does not auto-close the issue.
The event timeout begins after the navigation request returns; it does not interrupt a hung synchronous COM call. Existing broader COM apartment/threading behavior is unchanged. Please also verify selected-file restoration, tab reuse, foreground activation, and closing a tab during navigation on supported Windows builds.
Run the regression checks with: