diff --git a/docs/android-port.md b/docs/android-port.md
index 7724b45..cd6021e 100644
--- a/docs/android-port.md
+++ b/docs/android-port.md
@@ -348,6 +348,16 @@ runtime or the receipt is silently invisible — the service still runs, but not
to show, at most once, with no result read back: a refusal costs the notification and nothing else, which is
what the manifest's own comment on the permission says.
+**And a fourth correction, found by the notification refusing to come down.** "1 shell connected" outlived
+the shell, both ways a shell can close. A shell exiting on its own announced `SessionEnded` from inside its
+run's own finally block — where the run task is by definition not yet complete, so the
+`LiveSessionCount` the keep-alive reads from that event still counted the session that had just ended, and
+nothing fired afterwards to correct it. A tab closed by hand announced nothing at all, by a recorded
+decision that assumed every subscriber was the closer. Both reversed in `TerminalWorkspace`: the end is now
+announced from a continuation after the run has actually completed, and `CloseSessionAsync` announces too,
+after its own drain — the event's remark carries the reversal, and `SessionEnded`'s subscribers were all
+already "reconcile to reality" handlers for which a second announcement is harmless.
+
### Phone first
About 360dp wide. The tablet route was cheaper — a landscape tablet is close to the existing 880×560 minimum
diff --git a/docs/manual-checks.md b/docs/manual-checks.md
index 6ea93fd..1d07b81 100644
--- a/docs/manual-checks.md
+++ b/docs/manual-checks.md
@@ -2130,13 +2130,19 @@ foreground service is protecting), wait thirty seconds with the shell doing noth
**Pass:** the notification stayed up the whole time, and the shell is exactly where it was — same scrollback,
same prompt — with typing reaching the host immediately. Exit the shell.
-**Pass:** the notification goes with it, once nothing else is open.
+**Pass:** the notification goes with it, once nothing else is open. Open another shell and close it from the
+shells strip's ✕ instead of exiting — the notification comes down for that route too, which is the route
+that used to leave it up: a deliberate close announced nothing to the keep-alive at all, and a shell exiting
+on its own was announced while the count still included it.
**Failure means:** an upload that stalls with the screen off is the count not reaching
`SessionForegroundService`, and Android has stopped the process mid-transfer. A notification left up
afterwards is `ActivityChanged` not being subscribed — the other end of the same wire. A shell that has
disconnected on return is `MainWindowViewModel.TerminalSessionOpened` never reaching `SessionKeepAlive` — the
-service only ever heard about a shell *ending*, so it never came up for one in the first place.
+service only ever heard about a shell *ending*, so it never came up for one in the first place. A
+notification still saying "1 shell connected" after the shell is gone — by either route — is
+`TerminalWorkspace.SessionEnded` firing before the run completed, or a close not announcing; see
+`AnnounceEndedAsync` and the event's own remark.
### 14.6a A Files connection with nothing moving still survives backgrounding
diff --git a/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs b/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs
index 1f97d3b..374d6bc 100644
--- a/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs
+++ b/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs
@@ -46,8 +46,11 @@ internal sealed class SessionKeepAlive : IDisposable
this.activeTransfers = activeTransfers;
this.holdsFileSession = holdsFileSession;
- // Raised on whatever thread the pump unwound on, which is fine: starting and stopping a service is
- // a binder call and needs no particular thread. Nothing here touches the interface.
+ // Raised on whatever thread the workspace announced from — a continuation of the ended run, or the
+ // closer's own — which is fine: starting and stopping a service is a binder call and needs no
+ // particular thread. Nothing here touches the interface. That the announcement waits for the run to
+ // actually complete, and comes for deliberate closes too, is what makes reading LiveSessionCount
+ // from it honest — the event's own remark carries the stuck notification that taught us both.
workspace.SessionEnded += OnSessionEnded;
}
diff --git a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs
index 3eaae25..e81699f 100644
--- a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs
+++ b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs
@@ -1936,9 +1936,9 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
return;
}
- // Removed first, so the workspace's SessionEnded — which fires as the pump unwinds — finds no tab to
- // mark dead and does nothing. The alternative ordering leaves a window in which a tab that is on its
- // way out is repainted as disconnected.
+ // Removed first, so the workspace's SessionEnded — announced once the close below has fully drained
+ // — finds no tab to mark dead and does nothing here. The alternative ordering leaves a window in
+ // which a tab that is on its way out is repainted as disconnected.
var index = Tabs.IndexOf(tab);
Tabs.Remove(tab);
diff --git a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs
index 098b6b4..ef97e06 100644
--- a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs
+++ b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs
@@ -257,7 +257,8 @@ public sealed class TerminalWorkspace : IAsyncDisposable
}
///
- /// Raised with the session id when a shell ends on its own.
+ /// Raised with the session id once a session is over — its shell having ended on its own, or a
+ /// deliberate close having fully drained.
///
///
///
@@ -268,13 +269,23 @@ public sealed class TerminalWorkspace : IAsyncDisposable
/// the half of the interface Avalonia draws.
///
///
- /// Raised on whatever thread the pump finished on, which is a thread-pool thread. A handler that
- /// touches an observable collection has to marshal; this type has no toolkit to do it with, which is
- /// exactly why it does not try.
+ /// Raised only after the session's run task has completed, and that ordering is load-bearing.
+ /// It used to fire from inside the run's own finally block, where the task is by definition not yet
+ /// complete — so a handler reading still counted the session that had
+ /// just ended, which is how the phone's foreground notification went on saying "1 shell connected"
+ /// over nothing. See . Raised on a thread-pool continuation, or on the
+ /// closer's own thread; a handler that touches an observable collection has to marshal either way.
///
///
- /// Not raised by . That path already has a caller who knows the session is
- /// going, and telling it what it just asked for is how a tab close turns into a second tab close.
+ /// Raised by too, which reverses a recorded decision. The old
+ /// reasoning — the caller asked, so telling it is an echo — assumed every subscriber was the caller.
+ /// The phone's keep-alive is not: it hears this event to reconcile a notification with reality, and a
+ /// close that announced nothing left that notification claiming a shell that was gone. Every subscriber
+ /// treats the event as "reconcile" rather than "act" — a tab is marked dead if it is still there and
+ /// skipped if it is not — so a second announcement for a session that already announced its own end
+ /// (closing the tab of a shell that exited earlier) is deliberate and harmless. Shutdown is the one
+ /// close that stays silent: is tearing the subscribers down with the
+ /// sessions, and news nobody is left to hear is not news.
///
///
public event EventHandler? SessionEnded;
@@ -403,6 +414,10 @@ public sealed class TerminalWorkspace : IAsyncDisposable
sessions[sessionId] = new LiveSession(connection, pump, run);
}
+ // The announcement's own continuation — see AnnounceEndedAsync. Started after the entry is stored,
+ // so the containment check inside it can never run against a dictionary the session had not reached.
+ _ = AnnounceEndedAsync(sessionId, run);
+
return sessionId;
}
@@ -574,6 +589,14 @@ public sealed class TerminalWorkspace : IAsyncDisposable
{
// Expected on the ordinary path: disposing the pump cancels its run.
}
+
+ // After the drain, so a handler reading LiveSessionCount sees this session already gone — the
+ // event's own remark carries why a deliberate close is announced at all, and why shutdown is not:
+ // DisposeAsync sets the flag before its closing loop, and is dismantling every subscriber anyway.
+ if (Volatile.Read(ref disposed) == 0)
+ {
+ SessionEnded?.Invoke(this, new TerminalSessionEndedEventArgs(sessionId));
+ }
}
///
@@ -694,21 +717,51 @@ public sealed class TerminalWorkspace : IAsyncDisposable
// the pump unwinding, which is why CloseSessionAsync needs no call of its own — and why this
// must not do any work: it is running on a thread-pool thread inside DisposeAsync's loop when
// the application is closing.
+ //
+ // SessionEnded is deliberately NOT raised from here, and it used to be — see
+ // AnnounceEndedAsync for what was wrong with that.
ConnectionLog?.Closed(sessionId, clock.GetUtcNow());
+ }
+ }
- // Only when the session is still one this workspace knows about. CloseSessionAsync removes the
- // entry before it disposes the pump, so a tab the user closed does not come back as news.
- bool announce;
+ /// Announces a session's end once its run task has actually completed.
+ ///
+ ///
+ /// A continuation rather than a line in 's finally, and the difference is
+ /// what a handler sees. Inside that finally the run task is not yet complete — a finally is part of the
+ /// task — so , which counts incomplete runs, still included the session
+ /// that had just ended. The phone's keep-alive answers this event by reading exactly that count, and
+ /// reconciled its foreground notification to "1 shell connected" over a shell that was gone, with
+ /// nothing left to fire afterwards and correct it. By the time an await on the run resumes, the task is
+ /// complete and the count is honest.
+ ///
+ ///
+ /// The containment check keeps the deliberate paths out of this route:
+ /// removes the entry before it disposes the pump, and makes its own announcement after its own drain.
+ ///
+ ///
+ private async Task AnnounceEndedAsync(uint sessionId, Task run)
+ {
+ try
+ {
+ await run.ConfigureAwait(false);
+ }
+ catch (Exception exception) when (exception is not OutOfMemoryException)
+ {
+ // The run's faults belong to whoever drains it — CloseSessionAsync, on the deliberate path.
+ // This continuation cares only that the run is over, however it got there.
+ }
- lock (sessionGate)
- {
- announce = sessions.ContainsKey(sessionId);
- }
+ bool announce;
- if (announce)
- {
- SessionEnded?.Invoke(this, new TerminalSessionEndedEventArgs(sessionId));
- }
+ lock (sessionGate)
+ {
+ announce = sessions.ContainsKey(sessionId);
+ }
+
+ if (announce)
+ {
+ SessionEnded?.Invoke(this, new TerminalSessionEndedEventArgs(sessionId));
}
}
diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs
index 52dca61..bed2e44 100644
--- a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs
+++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs
@@ -217,12 +217,15 @@ public sealed class TerminalWorkspaceTests
}
///
- /// The event the tab strip listens to, so a dot can go out the moment a shell exits rather than at the
- /// next thing that happens to repaint. Raised only when the session ended on its own: a tab the user
- /// closed has a caller who already knows, and telling it would turn one close into two.
+ /// The event the tab strip and the phone's keep-alive listen to, so a dot can go out — and a foreground
+ /// notification can come down — the moment a shell exits rather than at the next thing that happens to
+ /// repaint. The count captured inside the handler is the sharper half of this test: the announcement
+ /// used to fire from inside the run's own finally block, where the run task is not yet complete, so
+ /// LiveSessionCount read from the handler still said 1 — and the phone's notification went on
+ /// claiming a shell that was gone, with nothing left to fire and correct it.
///
[Fact]
- public async Task ASessionEndingOnItsOwnIsAnnounced()
+ public async Task ASessionEndingOnItsOwnIsAnnounced_AfterTheCountStoppedIncludingIt()
{
// A shell with no output to give: its first read returns 0, which is a remote closing the channel,
// so the pump finishes with nobody asking it to.
@@ -231,10 +234,12 @@ public sealed class TerminalWorkspaceTests
await using var workspace = CreateWorkspace(connections);
var ended = new List();
+ var liveAtAnnouncement = -1;
workspace.SessionEnded += (_, e) =>
{
lock (ended)
{
+ liveAtAnnouncement = workspace.LiveSessionCount;
ended.Add(e.SessionId);
}
};
@@ -249,18 +254,33 @@ public sealed class TerminalWorkspaceTests
return ended.Contains(sessionId);
}
});
+
+ lock (ended)
+ {
+ liveAtAnnouncement.ShouldBe(0, "the announcement must wait for the run to actually complete");
+ }
}
- ///
+ ///
+ /// The reversal of a recorded decision, and the event's own remark carries why: a close used to be
+ /// announced to nobody, on the theory that the caller already knew — but the phone's keep-alive is not
+ /// the caller, and a close it never heard about left the foreground notification claiming a shell that
+ /// was gone. Announced once, after the drain, so the count a handler reads is already honest.
+ ///
[Fact]
- public async Task ClosingASessionIsNotAnnouncedBack()
+ public async Task ClosingASessionIsAnnounced_OnceItHasDrained()
{
var connections = new FakeConnectionFactory();
await using var workspace = CreateWorkspace(connections);
var announcements = 0;
- workspace.SessionEnded += (_, _) => Interlocked.Increment(ref announcements);
+ var liveAtAnnouncement = -1;
+ workspace.SessionEnded += (_, _) =>
+ {
+ liveAtAnnouncement = workspace.LiveSessionCount;
+ Interlocked.Increment(ref announcements);
+ };
var sessionId = await workspace.OpenSessionAsync(
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
@@ -268,12 +288,15 @@ public sealed class TerminalWorkspaceTests
await workspace.CloseSessionAsync(sessionId);
Volatile.Read(ref announcements)
- .ShouldBe(0, "a close the caller asked for is not news to report back to it");
+ .ShouldBe(1, "a close is news to the keep-alive even though it is an echo to the closer");
+ liveAtAnnouncement.ShouldBe(0, "announced after the drain, so the count already excludes it");
}
///
/// Disposal is the other path that closes sessions, because it is process shutdown. Asserted so
- /// that the SSH connections are known to be released rather than assumed to be.
+ /// that the SSH connections are known to be released rather than assumed to be — and that these closes,
+ /// unlike a deliberate one, are announced to nobody: shutdown is dismantling every subscriber along
+ /// with the sessions, and news nobody is left to hear is not news.
///
[Fact]
public async Task DisposingTheWorkspaceClosesEverySession()
@@ -282,6 +305,9 @@ public sealed class TerminalWorkspaceTests
var workspace = CreateWorkspace(connections);
+ var announcements = 0;
+ workspace.SessionEnded += (_, _) => Interlocked.Increment(ref announcements);
+
await workspace.OpenSessionAsync(
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
await workspace.OpenSessionAsync(
@@ -292,6 +318,7 @@ public sealed class TerminalWorkspaceTests
workspace.LiveSessionCount.ShouldBe(0);
connections.Connections.Count.ShouldBe(2);
connections.Connections.ShouldAllBe(connection => connection.IsDisposed);
+ Volatile.Read(ref announcements).ShouldBe(0, "shutdown closes are not announced");
}
// ---- Reattach ----