Skip to content

DoInvokes runs TryBeginInvoke actions before earlier InvokeAsync work from the same thread, so submission order isn't preserved #60

Closed

Description

@matt-edmondson

What's wrong

Invoker keeps two separate queues: BeginInvokeQueue for TryBeginInvoke, and TaskQueue for Invoke/InvokeAsync. DoInvokes (Invoker/Invoker.cs, ~lines 164–172) empties the whole BeginInvokeQueue first and only then empties TaskQueue:

while (BeginInvokeQueue.TryDequeue(out Action? action)) { action!(); }
while (TaskQueue.TryDequeue(out Task? task)) { task.RunSynchronously(); }

So on a single pump, work sent through TryBeginInvoke always runs before work sent earlier through InvokeAsync, even when one thread made both calls in that order.

Failure scenario

A worker thread runs:

_ = invoker.InvokeAsync(CreateTexture);   // submitted first
invoker.TryBeginInvoke(UseTexture);       // fire-and-forget follow-up

On the next DoInvokes(), UseTexture runs before CreateTexture, so it uses a resource that doesn't exist yet. A reproduction that makes both calls from one background thread and then pumps on the owner thread prints:

2: TryBeginInvoke (submitted second)
1: InvokeAsync (submitted first)

The TryBeginInvoke remarks only promise FIFO order among TryBeginInvoke actions. Nothing in the XML docs, README or CLAUDE.md warns that the two APIs don't keep order relative to each other. Callers reasonably expect a single marshalling queue to run work in submission order.

Suggested fix

Preferred: preserve submission order across both APIs. For example, tag each queued item with a shared Interlocked.Increment sequence number and have DoInvokes merge the two queues by sequence. Alternatively, route the Task path through the same queue as TryBeginInvoke.

If the split is deliberate, e.g. to keep TryBeginInvoke allocation-free, document on both TryBeginInvoke and DoInvokes (and in the README) that the two kinds of work are drained separately and aren't ordered relative to each other.

Acceptance: a test submitting InvokeAsync(A) and then TryBeginInvoke(B) from one thread observes A before B, or the documentation states that this isn't guaranteed.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Decision (maintainer, 2026-09-28)

    • Preserve submission order across both APIs. This is the preferred fix. InvokeAsync(A) followed by TryBeginInvoke(B) from one thread must run A before B, so documenting the split is not enough.
    • The mechanism is left to the implementer. Two options are:
      • a shared Interlocked.Increment sequence number, merged in DoInvokes
      • routing the Task path through the same queue

    Next reader: implement ordered draining in DoInvokes, and add the acceptance test (InvokeAsync(A) then TryBeginInvoke(B) from one thread, observe A before B). Update the TryBeginInvoke remarks to state the cross-API ordering guarantee.


    Generated by Claude Code

  2. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    Notes: The maintainer decision is to preserve submission order across both APIs; documenting the split is not enough. A shared Interlocked sequence number keeps TryBeginInvoke allocation-light. A single queue is simpler and makes #56's exception handling easier to get right in one place.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions