Fixes #55 Use non-blocking Post when publishing off the UI thread

[rwmcintosh]

What

EventPublisher.PublishEvent previously dispatched every listener via the blocking SynchronizationContext.Send. This change makes dispatch thread-aware:

• Published from the context thread (the UI thread) → keep using Send, so handlers run inline and any state they touch is updated before PublishEvent returns. This preserves the existing synchronous semantics (read-after-publish, ordering) for UI-thread publishers, and Send to the UI thread from the UI thread is effectively a direct call anyway.
• Published from any other thread (a background thread) → use the non-blocking Post, so the background thread is no longer blocked waiting on the UI thread.

The thread check is SynchronizationContext.Current == _context — "is the current thread the one this publisher dispatches to?" — computed once per call.

Why

Blocking the publisher on the UI thread for every listener can stall background work and risks UI-thread contention. Switching unconditionally to Post would have been a behavioral change for UI-thread publishers that rely on synchronous completion, so the dispatch path is chosen per call based on the publishing thread.

Tests

Added coverage for both branches:

• When_publishing_an_event_from_the_thread_that_owns_the_synchronization_context → asserts Send (not Post) and that the listener is notified.
• When_publishing_an_event_from_a_thread_that_does_not_own_the_synchronization_context → asserts Post (not Send) and notification.

A RecordingSynchronizationContext runs both paths synchronously to keep observations deterministic, and a helper runs each publish on a throwaway thread with a chosen ambient context so the runner thread's SynchronizationContext.Current is never mutated. All 317 tests pass locally.

Fixes #55

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Event delivery now uses the correct dispatch path depending on the current thread, improving reliability when events are raised from different contexts.
  • Listener handling continues to be protected so errors are managed consistently during event publishing.

• Tests

  • Expanded coverage for event publishing across same-thread and cross-thread scenarios.
  • Added checks to verify the correct dispatch method is used while still confirming listeners receive the event.

[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 46 minutes and 35 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: 8490ba2b-1f02-47bb-9235-159a6740f480

📥 Commits

Reviewing files that changed from the base of the PR and between 3372dc5 and 2a627f2.

📒 Files selected for processing (1)

• tests/OSPSuite.Utility.Tests/EventPublisherSpecs.cs

📝 Walkthrough

Walkthrough

EventPublisher.PublishEvent<T> is updated to check whether the caller's thread owns the stored SynchronizationContext and dispatches listener callbacks via Send (same thread) or Post (background thread). Tests gain a RecordingSynchronizationContext, a thread-helper, and a shared base class to verify both dispatch paths.

Changes

Thread-aware Send/Post dispatch in EventPublisher

Layer / File(s)	Summary
Thread-aware Send/Post dispatch logic src/OSPSuite.Utility/Events/EventPublisher.cs	PublishEvent<T> compares SynchronizationContext.Current to the stored _context to set an isOnContextThread flag, then dispatches each listener callback via _context.Send(...) when on the context thread or _context.Post(...) when on a background thread, wrapping execution in _exceptionManager.
Test infrastructure and dispatch-path assertions tests/OSPSuite.Utility.Tests/EventPublisherSpecs.cs	Introduces RecordingSynchronizationContext (records Send/Post calls), a runOnThreadWithSynchronizationContext helper, and concern_for_publishing_an_event base class. Existing sync-context scenarios are rewritten to publish from a controlled thread and assert the correct dispatch flag, while still verifying listener notification.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Remove reliance on synchronous event delivery (publish-then-act ordering) MoBi#2418: Directly related—both concern EventPublisher.PublishEvent delivery semantics and the choice between synchronous and asynchronous event dispatch via SynchronizationContext.

Possibly related PRs

• Open-Systems-Pharmacology/OSPSuite.Utility#54: Modifies the same PublishEvent method in EventPublisher.cs, changing listener snapshot/pruning behaviour under the _listeners lock alongside dispatch.

Suggested reviewers

• msevestre

Poem

🐇 Hop, hop, which thread do you own?
If it's yours, I'll Send — no need to moan.
But a stranger's thread? I'll Post and flee,
Non-blocking events, as fast as can be!
The recording context writes down the score —
SendWasCalled or PostWasCalled, nothing more. ✉️

🚥 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 behavior change and references the linked issue.
Linked Issues check	✅ Passed	The PR implements the requested non-blocking Post path off the UI thread while preserving synchronous Send on the owning thread.
Out of Scope Changes check	✅ Passed	The changes stay focused on EventPublisher behavior and the supporting tests/refactor for that dispatch logic.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 55-publish-event-non-blocking-post

---

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]

It turns out that there is some merit to using this Post strategy now.

We also found that there are instances where the event publish is expecting to be synchronous, so in the majority of cases that's going to be preserved. I added an issue in MoBi to find and fix these instances, as well as find instances in core, pksim.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/OSPSuite.Utility/Events/EventPublisher.cs (1)

40-56: 🎯 Functional Correctness | 🟠 Major

Use an explicit thread-affinity check here.

SynchronizationContext.Current == _context only compares instances; another thread can install the same context and incorrectly take the blocking Send path. Capture the owning thread/dispatcher explicitly instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/OSPSuite.Utility/Events/EventPublisher.cs` around lines 40 - 56, The
thread-affinity check in EventPublisher currently relies on
SynchronizationContext.Current == _context, which can misidentify the calling
thread if the same context instance is installed elsewhere. Update the
publish/dispatch logic in EventPublisher to use an explicit owner-thread or
dispatcher affinity check captured with _context, and use that check to decide
between _context.Send and _context.Post. Keep the receiver dispatch path and
_exceptionManager.Execute behavior unchanged, but base the blocking/non-blocking
choice on the owning thread identity rather than context instance equality.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/OSPSuite.Utility.Tests/EventPublisherSpecs.cs`:
- Around line 54-55: The worker-thread join in the EventPublisherSpecs test is
unbounded and can hang the suite if PublishEvent blocks or deadlocks. Update the
test around thread.Start and thread.Join to use a bounded join with a timeout
and fail the test explicitly if the worker does not finish, so the blocking
behavior is still verified without risking an indefinite stall.

---

Outside diff comments:
In `@src/OSPSuite.Utility/Events/EventPublisher.cs`:
- Around line 40-56: The thread-affinity check in EventPublisher currently
relies on SynchronizationContext.Current == _context, which can misidentify the
calling thread if the same context instance is installed elsewhere. Update the
publish/dispatch logic in EventPublisher to use an explicit owner-thread or
dispatcher affinity check captured with _context, and use that check to decide
between _context.Send and _context.Post. Keep the receiver dispatch path and
_exceptionManager.Execute behavior unchanged, but base the blocking/non-blocking
choice on the owning thread identity rather than context instance equality.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 59003ffe-a4cf-4f1b-9f6e-0b9708860f2f

📥 Commits

Reviewing files that changed from the base of the PR and between 7313f77 and 3372dc5.

📒 Files selected for processing (2)

• src/OSPSuite.Utility/Events/EventPublisher.cs
• tests/OSPSuite.Utility.Tests/EventPublisherSpecs.cs