diff --git a/binding/SkiaSharp/SKManagedStream.cs b/binding/SkiaSharp/SKManagedStream.cs index fcaa5f70bae..60ae9a86bac 100644 --- a/binding/SkiaSharp/SKManagedStream.cs +++ b/binding/SkiaSharp/SKManagedStream.cs @@ -11,10 +11,11 @@ public class SKManagedStream : SKAbstractManagedStream private bool isAsEnd; private bool disposeStream; - private bool wasCopied; - private WeakReference parent; - private WeakReference child; + // Lazily-created snapshot for duplicate/fork. Stored in native memory + // via SKData (ref-counted), so multiple duplicates share one native + // byte buffer with no additional managed byte-buffer allocations per duplicate. + private SKData snapshotData; public SKManagedStream (Stream managedStream) : this (managedStream, false) @@ -56,28 +57,11 @@ protected override void Dispose (bool disposing) => protected override void DisposeManaged () { - var childStream = child?.Target as SKManagedStream; - var parentStream = parent?.Target as SKManagedStream; - - if (childStream != null && parentStream != null) { - // remove this stream from the list by connecting the parent with the child - childStream.parent = parent; - parentStream.child = child; - } else if (childStream != null) { - // transfer ownership to child - childStream.parent = null; - } else if (parentStream != null) { - // transfer ownership back to parent - parentStream.child = null; - parentStream.wasCopied = false; - parentStream.disposeStream = disposeStream; - - disposeStream = false; + if (snapshotData != null) { + snapshotData.Dispose (); + snapshotData = null; } - parent = null; - child = null; - if (disposeStream && stream != null) { stream.Dispose (); stream = null; @@ -113,15 +97,11 @@ private IntPtr OnReadManagedStream (IntPtr buffer, IntPtr size) protected internal override IntPtr OnRead (IntPtr buffer, IntPtr size) { - VerifyOriginal (); - return OnReadManagedStream (buffer, size); } protected internal override IntPtr OnPeek (IntPtr buffer, IntPtr size) { - VerifyOriginal (); - if (!stream.CanSeek) { return (IntPtr)0; } @@ -133,8 +113,6 @@ protected internal override IntPtr OnPeek (IntPtr buffer, IntPtr size) protected internal override bool OnIsAtEnd () { - VerifyOriginal (); - if (!stream.CanSeek) { return isAsEnd; } @@ -143,22 +121,16 @@ protected internal override bool OnIsAtEnd () protected internal override bool OnHasPosition () { - VerifyOriginal (); - return stream.CanSeek; } protected internal override bool OnHasLength () { - VerifyOriginal (); - return stream.CanSeek; } protected internal override bool OnRewind () { - VerifyOriginal (); - if (!stream.CanSeek) { return false; } @@ -168,8 +140,6 @@ protected internal override bool OnRewind () protected internal override IntPtr OnGetPosition () { - VerifyOriginal (); - if (!stream.CanSeek) { return (IntPtr)0; } @@ -178,8 +148,6 @@ protected internal override IntPtr OnGetPosition () protected internal override IntPtr OnGetLength () { - VerifyOriginal (); - if (!stream.CanSeek) { return (IntPtr)0; } @@ -188,8 +156,6 @@ protected internal override IntPtr OnGetLength () protected internal override bool OnSeek (IntPtr position) { - VerifyOriginal (); - if (!stream.CanSeek) { return false; } @@ -199,8 +165,6 @@ protected internal override bool OnSeek (IntPtr position) protected internal override bool OnMove (int offset) { - VerifyOriginal (); - if (!stream.CanSeek) { return false; } @@ -210,46 +174,43 @@ protected internal override bool OnMove (int offset) protected internal override IntPtr OnCreateNew () { - VerifyOriginal (); - return IntPtr.Zero; } - protected internal override IntPtr OnDuplicate () + private SKData GetOrCreateSnapshot () { - VerifyOriginal (); + if (snapshotData != null) + return snapshotData; if (!stream.CanSeek) - return IntPtr.Zero; - - var newStream = new SKManagedStream (stream, disposeStream); - newStream.parent = new WeakReference (this); - - wasCopied = true; - disposeStream = false; - child = new WeakReference (newStream); - - stream.Position = 0; + return null; + + var pos = stream.Position; + try { + stream.Position = 0; + snapshotData = SKData.Create (stream, stream.Length); + } finally { + stream.Position = pos; + } - return newStream.Handle; + return snapshotData; } - protected internal override IntPtr OnFork () + protected internal override IntPtr OnDuplicate () { - VerifyOriginal (); - - var newStream = new SKManagedStream (stream, disposeStream); - - wasCopied = true; - disposeStream = false; + var data = GetOrCreateSnapshot (); + if (data == null) + return IntPtr.Zero; - return newStream.Handle; + return SkiaApi.sk_memorystream_new_with_skdata (data.Handle); } - private void VerifyOriginal () + protected internal override IntPtr OnFork () { - if (wasCopied) - throw new InvalidOperationException ("This stream was duplicated or forked and cannot be read anymore."); + var duplicate = OnDuplicate (); + if (duplicate != IntPtr.Zero) + SkiaApi.sk_stream_seek (duplicate, (IntPtr)stream.Position); + return duplicate; } } } diff --git a/tests/Tests/SkiaSharp/SKManagedStreamTest.cs b/tests/Tests/SkiaSharp/SKManagedStreamTest.cs index 87f70c17884..cdc466fae69 100644 --- a/tests/Tests/SkiaSharp/SKManagedStreamTest.cs +++ b/tests/Tests/SkiaSharp/SKManagedStreamTest.cs @@ -408,10 +408,8 @@ SKCodec CreateCodec(out IntPtr streamHandle) } [SkippableFact] - public void StreamCanBeDuplicatedButTheOriginalCannotBeRead() + public void StreamCanBeDuplicatedAndBothRemainReadable() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); @@ -420,18 +418,21 @@ public void StreamCanBeDuplicatedButTheOriginalCannotBeRead() var dupe = stream.Duplicate(); Assert.NotSame(stream, dupe); - Assert.IsType(dupe); + // duplicate starts at position 0 Assert.Equal(1, dupe.ReadByte()); Assert.Equal(2, dupe.ReadByte()); Assert.Equal(3, dupe.ReadByte()); - Assert.Throws(() => stream.ReadByte()); + // original remains readable at its previous position + Assert.Equal(4, stream.ReadByte()); + Assert.Equal(5, stream.ReadByte()); + + dupe.Dispose(); + stream.Dispose(); } [SkippableFact] - public void StreamCanBeForkedButTheOriginalCannotBeRead() + public void StreamCanBeForkedAndBothRemainReadable() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); @@ -439,33 +440,41 @@ public void StreamCanBeForkedButTheOriginalCannotBeRead() var dupe = stream.Fork(); Assert.NotSame(stream, dupe); - Assert.IsType(dupe); + // fork starts at the same position as the original Assert.Equal(3, dupe.ReadByte()); Assert.Equal(4, dupe.ReadByte()); - Assert.Throws(() => stream.ReadByte()); + // original remains readable at its previous position + Assert.Equal(3, stream.ReadByte()); + Assert.Equal(4, stream.ReadByte()); + + dupe.Dispose(); + stream.Dispose(); } [SkippableFact] - public void StreamCannotBeDuplicatedMultipleTimes() + public void StreamCanBeDuplicatedMultipleTimes() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); Assert.Equal(2, stream.ReadByte()); - var dupe = stream.Duplicate(); - Assert.Throws(() => stream.Duplicate()); + var dupe1 = stream.Duplicate(); + var dupe2 = stream.Duplicate(); - Assert.Equal(1, dupe.ReadByte()); + Assert.Equal(1, dupe1.ReadByte()); + Assert.Equal(1, dupe2.ReadByte()); + // original still works + Assert.Equal(3, stream.ReadByte()); + + dupe1.Dispose(); + dupe2.Dispose(); + stream.Dispose(); } [SkippableFact] - public void StreamCanBeDuplicatedMultipleTimesIfTheChildIsDestroyed() + public void StreamCanBeDuplicatedMultipleTimesWithChildDisposed() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); @@ -477,18 +486,23 @@ public void StreamCanBeDuplicatedMultipleTimesIfTheChildIsDestroyed() dupe1.Dispose(); + // can still duplicate after child is disposed var dupe2 = stream.Duplicate(); Assert.Equal(1, dupe2.ReadByte()); Assert.Equal(2, dupe2.ReadByte()); - Assert.Throws(() => stream.Duplicate()); + // can duplicate again — no limit + var dupe3 = stream.Duplicate(); + Assert.Equal(1, dupe3.ReadByte()); + + dupe2.Dispose(); + dupe3.Dispose(); + stream.Dispose(); } [SkippableFact] - public void FullOwnershipIsTransferredToTheChildIfTheParentIsDisposed() + public void DuplicatesAreIndependentAfterParentDisposed() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); @@ -498,32 +512,59 @@ public void FullOwnershipIsTransferredToTheChildIfTheParentIsDisposed() Assert.Equal(1, dupe1.ReadByte()); Assert.Equal(2, dupe1.ReadByte()); - Assert.Throws(() => stream.Duplicate()); - + // disposing parent does not affect the independent duplicate stream.Dispose(); + Assert.Equal(3, dupe1.ReadByte()); + var dupe2 = dupe1.Duplicate(); Assert.Equal(1, dupe2.ReadByte()); Assert.Equal(2, dupe2.ReadByte()); - Assert.Throws(() => dupe1.Duplicate()); - dupe1.Dispose(); dupe2.Dispose(); + // the original .NET stream should be disposed since disposeManagedStream was true Assert.Throws(() => dotnet.Position); } + [SkippableFact] + public void DuplicateStreamIsDisposed() + { + var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); + var stream = new SKManagedStream(dotnet, true); + Assert.Equal(1, stream.ReadByte()); + Assert.Equal(2, stream.ReadByte()); + + var dupe1 = stream.Duplicate(); + Assert.Equal(1, dupe1.ReadByte()); + Assert.Equal(2, dupe1.ReadByte()); + + stream.Dispose(); + + var dupe2 = dupe1.Duplicate(); + var handle = dupe2.Handle; + Assert.Equal(1, dupe2.ReadByte()); + Assert.Equal(2, dupe2.ReadByte()); + + Assert.True(SKObject.GetInstance(handle, out _)); + + dupe2.Dispose(); + Assert.False(SKObject.GetInstance(handle, out _)); + + dupe1.Dispose(); + } + [SkippableFact] public void DuplicateStreamIsCollected() { - SkipOnNonWindows(); + SkipOnMono(); var handle = DoWork(); CollectGarbage(); - Assert.False(SKObject.GetInstance(handle, out _)); + Assert.False(SKObject.GetInstance(handle, out _)); IntPtr DoWork() { @@ -536,8 +577,6 @@ IntPtr DoWork() Assert.Equal(1, dupe1.ReadByte()); Assert.Equal(2, dupe1.ReadByte()); - Assert.Throws(() => stream.Duplicate()); - stream.Dispose(); var dupe2 = dupe1.Duplicate(); @@ -549,10 +588,8 @@ IntPtr DoWork() } [SkippableFact] - public void MiddleDuplicateCanBeRemoved() + public void MultipleDuplicatesAreIndependent() { - SkipOnNonWindows(); - var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); var stream = new SKManagedStream(dotnet, true); Assert.Equal(1, stream.ReadByte()); @@ -561,26 +598,43 @@ public void MiddleDuplicateCanBeRemoved() var dupe1 = stream.Duplicate(); Assert.Equal(1, dupe1.ReadByte()); Assert.Equal(2, dupe1.ReadByte()); - Assert.Throws(() => stream.Duplicate()); - var dupe2 = dupe1.Duplicate(); + // can still duplicate the original + var dupe2 = stream.Duplicate(); Assert.Equal(1, dupe2.ReadByte()); Assert.Equal(2, dupe2.ReadByte()); - Assert.Throws(() => stream.Duplicate()); - Assert.Throws(() => dupe1.Duplicate()); + // can also duplicate the duplicate + var dupe3 = dupe1.Duplicate(); + Assert.Equal(1, dupe3.ReadByte()); + Assert.Equal(2, dupe3.ReadByte()); + + // disposing any one does not affect the others dupe1.Dispose(); - Assert.Throws(() => stream.Duplicate()); + Assert.Equal(3, stream.ReadByte()); + Assert.Equal(3, dupe2.ReadByte()); + Assert.Equal(3, dupe3.ReadByte()); dupe2.Dispose(); - var dupe3 = stream.Duplicate(); - Assert.Equal(1, dupe3.ReadByte()); - Assert.Equal(2, dupe3.ReadByte()); - Assert.Throws(() => stream.Duplicate()); + dupe3.Dispose(); + stream.Dispose(); + } + + [SkippableFact] + public void NonSeekableStreamDuplicateAndForkReturnNull() + { + var dotnet = new MemoryStream(new byte[] { 1, 2, 3, 4, 5 }); + var nonSeekable = new NonSeekableReadOnlyStream(dotnet); + var stream = new SKManagedStream(nonSeekable); + + Assert.Null(stream.Duplicate()); + Assert.Null(stream.Fork()); + + stream.Dispose(); } [SkippableFact] - public void DotNetStreamIsNotClosedPrematurely() + public void DotNetStreamIsClosedWhenSKManagedStreamIsDisposed() { var dotnet = CreateTestStream(); var stream = new SKManagedStream(dotnet, true); @@ -589,11 +643,15 @@ public void DotNetStreamIsNotClosedPrematurely() var dupe = stream.Duplicate(); Assert.Equal(0, dupe.Position); + // the snapshot has been taken, so the .NET stream is closed when + // the SKManagedStream is disposed (the duplicate is independent) stream.Dispose(); + Assert.Throws(() => dotnet.Position); + + // the duplicate is still usable after the parent is disposed Assert.Equal(0, dupe.Position); dupe.Dispose(); - Assert.Throws(() => dotnet.Position); } } }