Skip to content

Fix double dispose in OrderBy, Shuffle, Shuffle.SkipTake#241

Merged
neuecc merged 1 commit intoCysharp:mainfrom
apkd:fix-double-dispose-orderby-shuffle
Feb 12, 2026
Merged

Fix double dispose in OrderBy, Shuffle, Shuffle.SkipTake#241
neuecc merged 1 commit intoCysharp:mainfrom
apkd:fix-double-dispose-orderby-shuffle

Conversation

@apkd
Copy link
Copy Markdown
Contributor

@apkd apkd commented Feb 3, 2026

Hi, this is a followup to #192 - I came across the same problem in OrderBy, and it looks like Shuffle and Shuffle.SkipTake are also affected. I applied pretty much the same fix as before. I will test other operators when I have some time.

@apkd
Copy link
Copy Markdown
Contributor Author

apkd commented Feb 3, 2026

I also wrote some tests to reproduce the situation where double dispose matters. Let me know if you want them. These tests don't pass until the fixes are applied.

diff --git forkSrcPrefix/tests/ZLinq.Tests/Linq/OrderByTest.cs forkDstPrefix/tests/ZLinq.Tests/Linq/OrderByTest.cs
index b2d08bbd51fc6532d4956ee901124bdbdea0bbbc..977c37b64a8bc85d6e19b1881ba884c2b54575cd 100644
--- forkSrcPrefix/tests/ZLinq.Tests/Linq/OrderByTest.cs
+++ forkDstPrefix/tests/ZLinq.Tests/Linq/OrderByTest.cs
@@ -448,6 +448,13 @@ And Immortality.".Split([' ', '\n', '\r', '-'], StringSplitOptions.RemoveEmptyEn
         Assert.Equal(Enumerable.Range(0, 10).Reverse(), ordered);
     }
 
+    [Fact]
+    public void DoesNotDoubleDisposeTest()
+    {
+        // should not throw
+        new ValueEnumerableThatThrowsOnDoubleDispose().AsValueEnumerable().OrderBy(x => x).ToArray();
+    }
+
     private class ExtremeComparer : IComparer<int>
     {
         public int Compare(int x, int y)
diff --git forkSrcPrefix/tests/ZLinq.Tests/Linq/ReverseTest.cs forkDstPrefix/tests/ZLinq.Tests/Linq/ReverseTest.cs
index ac8e433fa6d07dfa8f1730724e02c855c66c75ab..9c7e0e8a56a380fd104b45af0bf7d8d2822b44dd 100644
--- forkSrcPrefix/tests/ZLinq.Tests/Linq/ReverseTest.cs
+++ forkDstPrefix/tests/ZLinq.Tests/Linq/ReverseTest.cs
@@ -218,6 +218,13 @@ public class ReverseTest
         // Dispose properly
         reversed.Dispose();
     }
+
+    [Fact]
+    public void DoesNotDoubleDisposeTest()
+    {
+        // should not throw
+        new ValueEnumerableThatThrowsOnDoubleDispose().AsValueEnumerable().Reverse().ToArray();
+    }
 }
 
 #if !NET10_0_OR_GREATER
diff --git forkSrcPrefix/tests/ZLinq.Tests/Linq/ShuffleSkipTest.cs forkDstPrefix/tests/ZLinq.Tests/Linq/ShuffleSkipTest.cs
index 3cdfd429c7916613ede61daff4219c5756413443..a8fa2d9be76597863fbc31d4aeaaa298501b8908 100644
--- forkSrcPrefix/tests/ZLinq.Tests/Linq/ShuffleSkipTest.cs
+++ forkDstPrefix/tests/ZLinq.Tests/Linq/ShuffleSkipTest.cs
@@ -209,4 +209,10 @@ public class ShuffleSkipTest
         source.Except(source.AsValueEnumerable().Shuffle().SkipLast(size).ToList()).Count().ShouldBe(size);
     }
 
+    [Fact]
+    public void DoesNotDoubleDisposeTest()
+    {
+        // should not throw
+        new ValueEnumerableThatThrowsOnDoubleDispose().AsValueEnumerable().Shuffle().Skip(1).ToArray();
+    }
 }
diff --git forkSrcPrefix/tests/ZLinq.Tests/Linq/ShuffleTest.cs forkDstPrefix/tests/ZLinq.Tests/Linq/ShuffleTest.cs
index 8b4ed368a97cbe5a460d60ba390f52f249482e28..c73ad84e58f271e134cfc0cb61a080ce01965893 100644
--- forkSrcPrefix/tests/ZLinq.Tests/Linq/ShuffleTest.cs
+++ forkDstPrefix/tests/ZLinq.Tests/Linq/ShuffleTest.cs
@@ -168,6 +168,13 @@ public class ShuffleTest
         collected.OrderBy(x => x).ShouldBe(original.OrderBy(x => x));
     }
 
+    [Fact]
+    public void DoesNotDoubleDisposeTest()
+    {
+        // should not throw
+        new ValueEnumerableThatThrowsOnDoubleDispose().AsValueEnumerable().Shuffle().ToArray();
+    }
+
     public struct TestStruct
     {
         public int Value;
diff --git forkSrcPrefix/tests/ZLinq.Tests/TestUtil.cs forkDstPrefix/tests/ZLinq.Tests/TestUtil.cs
index b4befda3a2e8487e92ebb854ddd8b975c5e0a66c..96af14bc00a6e0a65bd74fdf213e65a5fc59be5a 100644
--- forkSrcPrefix/tests/ZLinq.Tests/TestUtil.cs
+++ forkDstPrefix/tests/ZLinq.Tests/TestUtil.cs
@@ -228,3 +228,55 @@ public class DelegateBasedCollection<T> : ICollection<T>
     public IEnumerator<T> GetEnumerator() => GetEnumeratorWorker();
     IEnumerator IEnumerable.GetEnumerator() => NonGenericGetEnumeratorWorker();
 }
+
+public struct ValueEnumerableThatThrowsOnDoubleDispose
+    : IValueEnumerable<ValueEnumerableThatThrowsOnDoubleDispose, int>, IValueEnumerator<int>
+{
+    // This struct demonstrates a scenario where Dispose() affects external state, and
+    // it is forbidden to call it twice.
+    // A real-world equivalent would be ValueEnumerables based on Unity's ListPool<T>,
+    // which throws an exception when the list is returned to the pool more than once.
+
+    readonly bool[] disposeSentinel = new bool[1];
+    int index = -1;
+
+    public ValueEnumerableThatThrowsOnDoubleDispose() { }
+
+    public ValueEnumerable<ValueEnumerableThatThrowsOnDoubleDispose, int> AsValueEnumerable()
+        => new(this);
+
+    void IDisposable.Dispose()
+    {
+        if (disposeSentinel[0])
+            throw new ObjectDisposedException(nameof(ValueEnumerableThatThrowsOnDoubleDispose));
+
+        disposeSentinel[0] = true;
+    }
+
+    bool IValueEnumerator<int>.TryGetNext(out int current)
+    {
+        if (index++ < 10)
+        {
+            current = index;
+            return true;
+        }
+
+        current = 0;
+        return false;
+    }
+
+    bool IValueEnumerator<int>.TryGetNonEnumeratedCount(out int count)
+    {
+        count = 10;
+        return true;
+    }
+
+    bool IValueEnumerator<int>.TryGetSpan(out ReadOnlySpan<int> span)
+    {
+        span = default;
+        return false;
+    }
+
+    bool IValueEnumerator<int>.TryCopyTo(scoped Span<int> destination, Index offset)
+        => false;
+}

@neuecc
Copy link
Copy Markdown
Member

neuecc commented Feb 12, 2026

Thank you, I'll merge and apply it just like before.

@neuecc neuecc merged commit 1ac601f into Cysharp:main Feb 12, 2026
4 checks passed
@apkd apkd deleted the fix-double-dispose-orderby-shuffle branch February 12, 2026 12:41
IhateTrains pushed a commit to ParadoxGameConverters/ImperatorToCK3 that referenced this pull request Feb 13, 2026
Updated [ZLinq](https://github.com/Cysharp/ZLinq) from 1.5.4 to 1.5.5.

<details>
<summary>Release notes</summary>

_Sourced from [ZLinq's
releases](https://github.com/Cysharp/ZLinq/releases)._

## 1.5.5

## What's Changed
* Update GH action benchmark results link by @​SnakyBeaky in
Cysharp/ZLinq#238
* chore(deps): bump actions/upload-artifact from 5.0.0 to 6.0.0 by
@​dependabot[bot] in Cysharp/ZLinq#239
* Fix OverflowException in Average() for int type by using long
accumulator by @​prozolic in Cysharp/ZLinq#240
* Fix double dispose in OrderBy, Shuffle, Shuffle.SkipTake by @​apkd in
Cysharp/ZLinq#241

## New Contributors
* @​SnakyBeaky made their first contribution in
Cysharp/ZLinq#238

**Full Changelog**:
Cysharp/ZLinq@1.5.4...1.5.5

Commits viewable in [compare
view](Cysharp/ZLinq@1.5.4...1.5.5).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=ZLinq&package-manager=nuget&previous-version=1.5.4&new-version=1.5.5)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants