Repository navigation
Ask the listener table whether a viewer holds the queue - #999
Merged
Merged
Conversation
RemoteInlineHost asked .NET's GetActiveTcpListeners whether anything held the viewer port, and skipped the connect when it said nothing did. Windows 10.0.26300 gives a listening socket's row in the table that list is read from a state of 0, which .NET reads as Unknown, so every listener an app opened was left out: a TcpListener's row there has dwState 0, and netstat shows the tray's own listener as CLOSED. On that build the tray took a viewer that held the queue for one that had gone, showed none of its snapshots, and in an accept-all would read the queue as empty. That failed sixteen tray core tests locally, among them the one that holds a delete back while the owner cannot be asked, and none on CI. It now asks ListenerTable, as ViewerClient and PiperClient do, which on Windows asks for the listeners alone and lists them on that build. A table that cannot be read still leaves the connect to decide. Two tests pin it: the host asks the table, and a table saying nobody holds the port is believed with no connect.
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.
The tray takes a viewer that owns the inline queue for one that has gone, on Windows 10.0.26300. Found because sixteen tray core tests failed on that machine and passed on CI.
Cause.
RemoteInlineHostdrives a queue a viewer owns. Before each exchange it asked .NET'sIPGlobalProperties.GetActiveTcpListeners()whether anything held the viewer port, and skipped the connect when nothing did. That list is every TCP row fromGetTcpTable, with the rows in the LISTEN state picked out.On Windows 10.0.26300,
GetTcpTablegives a listening socket's row adwStateof 0, which isn't a TCP state at all, so .NET reads it asUnknown. What I saw on that build:TcpListeneron loopback hasdwState0, and .NET reportsUnknownfor it.netstat -anodoesn't show it, and shows the running tray's listener on 3492 asCLOSED.GetExtendedTcpTablewith the listener class lists it.So the host said nobody held the port while a viewer did. With no connect made, the listing was empty and the owner counted as absent. The tray showed none of the viewer's snapshots, and an accept-all read the queue as empty. The failing tests included
AnOwnerThatDoesNotAnswerHoldsTheDeletes.Fix. Ask
ListenerTableinstead, which is whatViewerClientandPiperClientalready ask. On Windows it asks for the listeners alone, which that build lists correctly. A table that can't be read still counts as held, so the connect decides, which is what the old null meant.Tests.
TrackerSnapshotTest.TheOwnerIsLookedForInTheListenerTable: the host asks the table, and finds the fake viewer.TrackerSnapshotTest.NobodyInTheListenerTableIsNoOwner: a table saying nobody holds the port is believed, and the viewer is never connected to.main'sRemoteInlineHost, in a throwaway worktree, the first fails on that machine. The second passes there, because .NET's list already says nobody, but would fail on a correct Windows, where the old code connects.