fix: stop callers disposing the surface everyone shares - #4
Merged
Conversation
The transport tests got a surface with no size and no planes, and it was never about the test host: macOS bindings keep one managed peer per native object, and a server publishes into one recycled surface. So every frame a client delivers is the same managed instance the server writes through - and TryGetFrame told callers to dispose it. The first dispose zeroed that shared handle, and every later read came back empty. The polling loops in the tests and the sample disposed the previous frame each iteration, which made whether a run passed a matter of how the parity fell. Frames and acquired surfaces now belong to the library: both hold the peer so it is not finalized between calls, and both document that callers read the pixels rather than take the surface. Ownership is unchanged - owns:true still consumes the shim's retain, and the count stays flat across a frame loop. Also fix the plane helpers the second failure exposed. IOSurface reports no planes for a packed surface and its per-plane accessors raise NSGenericException there, so PlaneCount now answers 1 as it documents and PlaneInfo describes plane 0 from the surface itself. Closes #3
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
1 participant
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.
Fixes the two failing transport tests in #3.
Root cause: not the test host. macOS bindings keep one managed peer per native object, and a Syphon server publishes into a single recycled surface — so the surface a client delivers is the same managed instance the server writes through.
TryGetFramedocumented "dispose it once consumed", and the polling loops did; the first dispose zeroed that shared handle and every later read reported no size, no planes, no pixels. It reproduces in a plain local run, and passing was only ever a matter of how the loop parity fell (which is why the sample's runs stayed green).Changes
SyphonClient.TryGetFrame/SyphonServer.AcquireSurfacekeep the peer alive and document that the surface belongs to the library. Ownership is unchanged —owns: truestill consumes the shim's retain; retain count measured flat over 60 frames.IOSurfaceExtensions.PlaneCountreports 1 for a packed surface (IOSurface itself reports 0), andPlaneInfoanswers plane 0 from the surface instead of letting-[IOSurface widthOfPlaneAtIndex:]raiseNSGenericException.API compatibility: no signature changes; the zero-copy publish/receive paths are untouched. Code that disposed a frame still compiles — it was already breaking itself.
Verified locally: 8/8 tests pass across repeated runs (previously 2 failed), Release solution build clean with 0 warnings, and the sample's
probeandcrosstestboth pass.