Fix #55: dispatch events by thread identity, not context-instance equality

[rwmcintosh]

Problem

EventPublisher.PublishEvent chooses Send (synchronous, inline) vs Post (asynchronous, deferred) with SynchronizationContext.Current == _context — instance equality, as introduced in #56 (Fixes #55).

A single UI thread can own more than one SynchronizationContext instance (e.g. a manually-created one and the one the WinForms message loop installs/copies), so on the UI thread Current != _context and every UI-thread publish takes the Post (deferred) branch instead of Send.

Two consumer-side symptoms, both swallowed by ExceptionManager.Execute (so only visible under a debugger):

• Startup splash progress is deferred/batched (jumps to 100%, cross-thread touches of splash controls).
• NullReferenceException at app close: a ProjectClosedEvent is published and then Application.Exit() tears down the UI; the deferred handler runs after teardown.

Fix

Decide by thread identity, not context instance:

var publishingFromContextThread = ReferenceEquals(Thread.CurrentThread, _contextThread);

• Adds an explicit-thread constructor (SynchronizationContext, Thread contextThread, IExceptionManager). The existing 2-arg constructor delegates to it with Thread.CurrentThread — additive and backward-compatible. Use the explicit constructor when the context targets a thread other than the one constructing the publisher (e.g. a splash screen running on its own thread).
• Send is chosen only when we are the context thread, so it always runs inline (never blocks or marshals); a different thread always takes the non-blocking Post.

Tests

EventPublisherSpecs updated to thread semantics, plus new cases:

• publish from the context thread while a different SynchronizationContext is Current → still Send (the regression this fixes);
• explicit-thread constructor: on its thread → Send, off it → Post.

Refs #55.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added an overload to configure event dispatch using an explicit thread when creating the event publisher.
• Bug Fixes
  • Improved event delivery by choosing synchronous vs asynchronous handler dispatch based on the publisher’s owning thread.
  • Fixed cases where a changing current synchronization context could lead to an incorrect dispatch path.
• Tests
  • Expanded coverage for event publishing when invoked from the same thread vs a different thread.
  • Added regression checks to ensure listeners are notified correctly and that the expected dispatch mode is used.

[coderabbitai]

Warning

Review limit reached

@rwmcintosh, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 45 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 219b7828-7aec-4296-b76c-45e3be3b3c2d

📥 Commits

Reviewing files that changed from the base of the PR and between 2374675 and e28b72f.

📒 Files selected for processing (1)

• src/OSPSuite.Utility/Events/EventPublisher.cs

📝 Walkthrough

Walkthrough

EventPublisher now captures a construction thread and uses it to choose synchronous Send or asynchronous Post during event publishing. The tests cover same-thread publishing, a changed synchronization context, and explicit same-thread versus different-thread dispatch.

Changes

EventPublisher thread-aware dispatch

Layer / File(s)	Summary
Capture the owning thread src/OSPSuite.Utility/Events/EventPublisher.cs	EventPublisher stores a captured thread, adds an overload that accepts an explicit Thread, and delegates the existing constructor to it.
Dispatch by publishing thread src/OSPSuite.Utility/Events/EventPublisher.cs	PublishEvent<T> compares Thread.CurrentThread with the captured thread and routes callbacks through _context.Send(...) or _context.Post(...).
Dispatch specs tests/OSPSuite.Utility.Tests/EventPublisherSpecs.cs	The specs widen _exceptionManager, clarify the helper comment, add a swapped-context regression case, and cover same-thread versus different-thread publishing with an explicit captured thread.

Sequence Diagram(s)

sequenceDiagram
  participant Thread as Thread.CurrentThread
  participant EventPublisher
  participant SynchronizationContext
  participant Listener

  Thread->>EventPublisher: PublishEvent<T>(event)
  EventPublisher->>EventPublisher: compare current thread with captured thread
  alt same thread
    EventPublisher->>SynchronizationContext: Send(...)
  else different thread
    EventPublisher->>SynchronizationContext: Post(...)
  end
  SynchronizationContext->>Listener: invoke handlers

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Open-Systems-Pharmacology/OSPSuite.Utility#54: Modifies EventPublisher.PublishEvent<T> in the same area of event publication flow.
• Open-Systems-Pharmacology/OSPSuite.Utility#56: Updates the same Send/Post dispatch decision and matching specs.

Suggested reviewers

• Yuri05
• msevestre

Poem

A bunny caught a thread so bright,
Then picked Send close and Post in flight.
With whiskers twitching, calm and keen,
The events hopped through the dispatcher scene. 🐰

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: event dispatch now depends on thread identity instead of synchronization-context instance equality.
Linked Issues check	✅ Passed	The PR implements #55 by using Post from other threads while preserving Send on the captured UI thread.
Out of Scope Changes check	✅ Passed	The added constructor and test updates support the threading fix and stay within the issue's scope.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-55-eventpublisher-thread-identity

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[rwmcintosh]

@coderabbitai are you able to see my license?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@rwmcintosh I can’t see your private license or billing details from the PR context.

What I can help with is checking whether CodeRabbit appears to be active/configured correctly for this repository or PR. If you want, I can help troubleshoot that side. For the actual license status, you’d need to check your CodeRabbit account/org settings.

[msevestre]

I don't get it. What are we doing here now compare to the other version? Too much texts to understandw hat's going on for me

[rwmcintosh]

You can have many instances of a synchronization context that point to the same thread. The decision to dispatch by send or post is not the captured context, but the captured thread.

[rwmcintosh]

And the context instance can be changed on the thread, but its target will always be the UI thread.

[msevestre]

I don't understand the thread stuff. EventPublisher is a singletong that will capture the thread once. Are we assuming that the process where event puvlisher is created is always the right one?

[msevestre]

event publisher is a singleton. That can't be good. We will capture the thread once. doenst it defeat the purpose?

[rwmcintosh]

No because the thread is the UI thread. There should only be one. It is the same target thread that the SynchronizationContext.Current targets. We always want to know if the publishing thread is the UI thread. Comparing context doesn't tell us that because more than one context can target the UI thread.

The way I found this was that when I ran some sequence-important publishing, the Post was used which I did not expect. That's because in the Forms event handler (eg FormClosing in this case), the SyncrhonizationContext.Current instance had changed on the thread. It still targets the UI thread, just the instance had changed.

So what was happening was

• we capture the SynchronizationContext.Current when EventPublisher is resolved for the first time.
• at some point the SynchronizationContext.Current instance changes, maybe as a result of the event, or async task, or some other life-cycle.
• publish an event on the UI thread, but the SynchronizationContext.Current has changed so Post is used even though we are publishing from the UI thread.

What we really want to know is are we publishing from the UI thread.

[msevestre]

should we rename so that it actually says this ?uiThread vs contextThread
publishingFromContextThread => publishingFromUIThread?

[rwmcintosh]

Done. Cleaned up the comment a bit so it's less confusing.

[rwmcintosh]

I don't understand the thread stuff. EventPublisher is a singletong that will capture the thread once. Are we assuming that the process where event puvlisher is created is always the right one?

Yes, we are assuming that the first instantiation of event publisher is on the UI thread.

Edit: Singleton - so it should be the only one. Although, I also found that SplashScreen has its own UI thread, and I had to create a special one that targets that thread so the progress bar was using Send instead of Post

[msevestre]

yeah good like this. I do think that some of the cokmments are just too much with claude lol. I don't need a book for every method

[rwmcintosh]

yeah good like this. I do think that some of the cokmments are just too much with claude lol. I don't need a book for every method

I always have to tell him to stop narrating