stride3d / stride3d/stride

Priority queues return out-of-order values after removing an internal item

Open
#3,398 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
7.8k
Forks
1.2k
Avg merge
2d 17h
Merged PRs (30d)
49

Description

Release Type: GitHub source

Version: 3d034317171905c1889fd6b3820d648b2af73312

Platform(s): Reproduced on Linux using .NET SDK 10.0.100. This is managed collection code; no graphics or editor execution is required for the reproduction.

Describe the bug

PriorityQueue<T>.Remove and PriorityNodeQueue<T>.Remove can break priority order when removing an internal item. They replace the removed slot with the last element and only repair downward. When that replacement is smaller than its new parent, it must move upward instead.

To Reproduce

Run this against the Stride.Core queue implementation at the commit above:

var queue = new Stride.Core.Collections.PriorityQueue<int>();
foreach (var value in new[] { 0, 1, 4, 2, 5, 6, 7, 3 })
    queue.Enqueue(value);

queue.Remove(5);

while (!queue.Empty)
    Console.WriteLine(queue.Dequeue());

Actual output: 0, 1, 2, 4, 3, 6, 7.

Expected behavior

Output remains in priority order: 0, 1, 2, 3, 4, 6, 7.

The same input reproduces with PriorityNodeQueue<int> when removing the node returned by Enqueue(5). Both variants also fail under a descending custom comparer.

var queue = new Stride.Core.Collections.PriorityNodeQueue<int>();
Stride.Core.Collections.PriorityQueueNode<int>? nodeToRemove = null;
foreach (var value in new[] { 0, 1, 4, 2, 5, 6, 7, 3 })
{
    var node = queue.Enqueue(value);
    if (value == 5)
        nodeToRemove = node;
}

queue.Remove(nodeToRemove!);

while (!queue.Empty)
    Console.WriteLine(queue.Dequeue());

Screenshots

Not applicable; the reproduction is a console collection test.

Log and callstacks

The regression assertion fails at the fourth dequeued value:

Assert.Equal() Failure: Values differ
Expected: 3
Actual:   4

Original production sources: Total tests: 14; Passed: 8; Failed: 6
Patched production sources:  Total tests: 14; Passed: 14; Failed: 0

Additional context

These are active engine paths: Scheduler.Unschedule removes empty priority buckets, AnimationChannel.Fitting removes queued error nodes, and AssetBuilderService.RemoveBuildUnit removes queue nodes. The collection-level ordering failure is verified; I have not run the editor or claimed a particular visible scene failure.

A prepared patch with regression tests repairs upward when needed while preserving the existing heap representation and maintaining both node indices on swaps. It includes six new regression cases: the eight-item example under both comparer directions, plus deterministic shuffled insertion/removal coverage for both queue variants. Compiling the three real production files and the two existing test files in an isolated xUnit project produced 8 passed / 6 failed before, then 14 passed / 0 failed after. No collection implementation was mocked or rewritten in the harness. The ordinary repository test invocation did not produce a final result in the captured log, so I am not claiming a passing engine build or full repository suite.

AI disclosure: OpenAI Codex prepared this investigation, patch, and executable regression tests for falseee1.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the Stride.Core.Collections PriorityQueue and PriorityNodeQueue Remove entry points, then run the supplied reproduction and existing collection tests. Verify removal preserves priority order for internal items, both queue variants, ascending and descending comparers, and node-index updates; add or run regression coverage for the listed cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.