Skip to content

[Tizen.System.Session] Ensure no-op (un)registration works correctly - #7848

Merged
chanwoochoi merged 1 commit into
Samsung:API14from
metiulekm:sessiond-fix-API14
Sep 19, 2026
Merged

chanwoochoi merged 1 commit into
Samsung:API14from
metiulekm:sessiond-fix-API14

Conversation

@metiulekm

Copy link
Copy Markdown
Contributor

Description of Change

The C# bindings coalesce multiple C# callbacks into one C callback, which is registered with the first C# callback and unregistered with the last C# callback. These operations should only happen on non-null<>null edges, otherwise there might be multiple registrations, or we can attempt unregistering when there is no registration.

However C# delegate removal and combination operators is actually more flexible than expected, in particular C# accepts nulls in both cases. Additionally, removal allows unregistering callbacks that are not registered, which is a no-op. These cases mean that some operations can start from a null and end in a null. In this case, we will call C callback (un)registration incorrectly.

This commit adds null checks:

  • registrations check if the callback being registered is non-null,
    • if it is a non-null, the result will always be non-null,
    • if it is a null, it is safe to return because adding a null does not do anything,
  • unregistrations check if the existing callback list is non-null,
    • it being non-null is enough to eliminate null<>null edges,
    • if it is a null, it is safe to return because there is nothing to unregister anyway.

API Changes

None

The C# bindings coalesce multiple C# callbacks into one C callback,
which is registered with the first C# callback and unregistered with the
last C# callback. These operations should only happen on non-null<>null
edges, otherwise there might be multiple registrations, or we can
attempt unregistering when there is no registration.

However C# delegate removal and combination operators is actually more
flexible than expected, in particular C# accepts nulls in both cases.
Additionally, removal allows unregistering callbacks that are not
registered, which is a no-op. These cases mean that some operations can
start from a null and end in a null. In this case, we will call C
callback (un)registration incorrectly.

This commit adds null checks:
- registrations check if the callback being registered is non-null,
  - if it is a non-null, the result will always be non-null,
  - if it is a null, it is safe to return because adding a null does not
    do anything,
- unregistrations check if the existing callback list is non-null,
  - it being non-null is enough to eliminate null<>null edges,
  - if it is a null, it is safe to return because there is nothing to
    unregister anyway.
@github-actions github-actions Bot added the API14 Platform : Tizen 11.0 / TFM: net8.0-tizen11.0 label Sep 16, 2026
@JoonghyunCho

Copy link
Copy Markdown
Member

🤖 [AI Review]

Reviewed — no findings.

Scope checked:

  • Backport to API14 of the same change proposed for main in [Tizen.System.Session] Ensure no-op (un)registration works correctly #7849; fetched the full Session.cs at aaf6558 and confirmed the API14 file has the same 4 custom event accessors (AddUserWait, RemoveUserWait, SwitchUserWait, SwitchUserCompleted) at the same positions, so the 8 guard hunks apply identically and no event is left unguarded.
  • add path: value == null early-return precedes the _handler == null registration check, so += null no longer triggers a native register on a null→null edge; a non-null value guarantees a non-null result, preserving register-on-first-subscriber.
  • remove path: _handler == null early-return prevents -= x on an empty list from reaching UnregisterCallbackForEvent; removing a null or absent delegate from a non-null list leaves it non-null, so unregister still fires only on the true non-null→null edge.
  • Both guards are inside the existing per-event lock, so edge detection remains atomic with the native call.
  • RegisterCallbackForEvent throws via CheckError before _handler += value, so a failed native registration leaves the handler null and a later subscribe retries; unchanged and consistent with the new guard.
  • Semantics now match the compiler-generated accessor behavior (+= null, -= null, removing an absent delegate are no-ops); no public API surface change.

No 🔴 critical issues, no 🟡 suggestions to flag.


Automated review — final merge decision rests with human reviewers.

@chanwoochoi
chanwoochoi merged commit ec70fe0 into Samsung:API14 Sep 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

API14 Platform : Tizen 11.0 / TFM: net8.0-tizen11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants