ISSUE-4417: FuriEventFlag: furi_event_flag_clear() notifies the event loop before the ISR-deferred clear is applied

[herbenderbler]

What's New

Problem (fixes #4417)

furi_event_flag_clear() has the same deferred-operation/early-notify defect on its ISR path that #4336 reported for furi_event_flag_set() (fixed by #4358). When furi_event_flag_clear() is called from interrupt context and an app subscribes via furi_event_loop_subscribe_event_flag() with FuriEventLoopEventOut, the event loop is notified before the clear is applied, evaluates the level against stale bits, and drops the event permanently.

This defect is latent: every in-tree caller of furi_event_flag_clear() runs in task context, so the ISR branch is currently unreachable. It is still worth fixing because furi_event_flag_clear() is part of the public FAP API (api_symbols.csv), so any external app or future in-firmware ISR can hit it — and with #4358 documenting furi_event_flag_set() as ISR-safe, the symmetric expectation for clear is natural. Filed as #4417, split out of #4358 per the discussion there.

Cause

FreeRTOS's xEventGroupClearBitsFromISR() does not clear the bits in the ISR — it is a thin wrapper around xTimerPendFunctionCallFromISR() that queues the clear for the timer daemon. The old code then called furi_event_loop_link_notify(..., FuriEventLoopEventOut) immediately, from the ISR. The event loop thread (priority 16) outranks the timer daemon (priority 2), so it woke first, evaluated the FuriEventLoopEventOut level while the bits were still uncleared, found the level false, and dropped the item from its waiting list. The daemon then applied the clear with no further notification, so the event was lost entirely. Not a narrow race — the deterministic outcome given the priority gap.

Fix

Notify only after the bits are actually cleared. Since xEventGroupClearBitsFromISR() is literally xTimerPendFunctionCallFromISR(vEventGroupClearBitsCallback, ...), the fix swaps in our own pended callback that:

1. calls xEventGroupClearBits()
2. then calls furi_event_loop_link_notify(..., FuriEventLoopEventOut) if any of the requested bits were actually set

By the time the event loop runs, the bits are cleared and the level evaluation observes the correct value. Everything else is preserved: the timer command queue is unchanged (if it is full, FuriStatusErrorResource is still returned), the pre-existing portYIELD_FROM_ISR(pdTRUE) and its ordering rationale (FreeRTOS-Kernel#93) still apply unchanged — the pended callback goes through the same timer command queue as the stock deferred clear, so relative ordering of ISR-issued event-group operations is FIFO-preserved. The ISR-path return value is still the flag state read at call time. Task-context behaviour is unchanged.

One deliberate difference from the set-side callback in #4358, found while verifying the approach: #4417 suggests the callback must keep the instance alive because "xEventGroupClearBits() can yield internally via xTaskResumeAll()" — that turns out not to be true. Unlike xEventGroupSetBits(), xEventGroupClearBits() is a plain critical section (lib/FreeRTOS-Kernel/event_groups.c): clearing bits can never satisfy a waiter, so it cannot wake or yield to a task that might free the flag. The callback still wraps both calls in vTaskSuspendAll()/xTaskResumeAll(), but for a different (and accurately commented) reason: it keeps clear + notify atomic with respect to other tasks, matching the task-context path, which performs both under FURI_CRITICAL, and keeps the two pended callbacks structurally identical once #4358 lands.

Relationship to #4358: independent change, branched off dev; neither PR depends on the other to build or pass. Both touch furi/core/event_flag.c, so whichever merges second will need a trivial conflict resolution (the two added includes, and two separate callback insertions).

Regression test

None added in this PR — deliberately. Exercising this path requires running furi_event_flag_clear() in ISR context, and the only in-tree mechanism for that is the furi_hal_interrupt_trigger_pending() helper plus the ISR test harness introduced by #4358, which has not merged yet. Duplicating that infrastructure here would guarantee API-CSV and test-file conflicts between the two PRs.

Once #4358 lands, test_furi_event_loop_event_flag_from_isr() is a direct template: subscribe FuriEventLoopEventOut instead of FuriEventLoopEventIn, pre-set the bits, and call furi_event_flag_clear() from the software-pending LPTIM2 ISR. I will follow up with that test in this PR (if still open) or a successor.

API surface

No change to the public FAP API. No entry is added, removed or changed in either CSV — targets/f7/api_symbols.csv and targets/f18/api_symbols.csv are untouched and stay at version 88.0, in lockstep with dev (fbt reports API version 88.0 is up to date for both targets). The only header change is a doc comment on furi_event_flag_clear().

Verification

Verified on physical hardware (Flipper Zero, f7), gated on info device reporting the exact fix commit:

Check	Result
firmware.commit.hash / firmware.commit.dirty	f5033d35 / false — device runs exactly the committed fix
firmware.api.major/minor	88.0
Full on-device unit_tests suite	PASSED — 329 tests, Failed tests: 0, 156.9 s. The runner's non-failing Leaked stat read 2440 bytes (it varies run-to-run: an earlier pass of the same diff read 3104); not attributable to this diff (no allocations added, and the modified ISR branch is unreachable in the suite).

The suite exercises the task-context furi_event_flag_clear() path (the FuriApiLock re-lock in the RPC tests and the BT service both sit on it) and the event-loop event-flag subscriptions in test_furi_event_loop, confirming no regression in the paths this change touches. The ISR branch itself has no in-tree reproducer yet — see Regression test above; on-device A/B of the ISR path will come with the follow-up test.

Also verified:

• ./fbt FIRMWARE_APP_SET=unit_tests firmware_all builds clean (f7); this is the image flashed for the run above (via flash_usb_full).
• ./fbt TARGET_HW=18 firmware_all builds clean (f18 is built by CI).

Documentation

• API comment in furi/core/event_flag.h — states that furi_event_flag_clear() is ISR-safe and that with a FuriEventLoopEventOut subscription the event loop is notified only after the clear is applied. (The corresponding furi_event_loop.h subscribe-side note is left to ISSUE-4336: EventLoop receives callbacks one event late when setting FuriEventFlag from interrupts #4358 to avoid a doc-comment collision.)

Checklist (For Reviewer)

• PR has description of feature/bug or link to Confluence/Jira task
• Description contains actions to verify feature/bugfix
• I've built this code, uploaded it to the device and verified feature/bugfix