Uh oh!
There was an error while loading. Please reload this page.
[single] Refactor invoke_thread to reduce goto statements - #663
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Refactor invoke_thread to reduce goto statements and simplify control flow Signed-off-by: hyunil park <hyunil46.park@samsung.com>
myungjoo-bot
left a comment
There was a problem hiding this comment.
Automated review (transcribed from an AI review agent's report; please verify before acting).
Summary: The PR removes the wait_for_next label from invoke_thread, renames exit to exit_thread, hoists single_h->status = status above the error/JOIN check, and turns the success path into an else branch. I traced every path in old vs new: the mutex is held on entry to every goto exit_thread, after the re-lock following __invoke, and on natural loop exit, and exit_thread unlocks exactly once. The g_cond_wait loop, invoking flag, output freeing on error/join vs __process_output on success, the alloc_output-guarded cleanup, broadcasts, and NULL in/out handling are unchanged. Merges cleanly onto current main. Existing tests (invoke_timeout, parallel_runs, close_while_running) cover all three thread paths. Minor comments only.
- [Low] Small semantic change not mentioned —
c/src/ml-api-inference-single.c:545:single_h->status = statusis now also stored whenstate == JOIN_REQUESTEDafter__invoke. Harmless (a still-waiting_ml_single_invoke_internalgets the real result instead of a stale one), but the PR is described as a pure refactor. Either keep the assignment where it was (just before theif (single_h->state == RUNNING)) or mention the change in the commit message. - [Low] Inaccurate comment —
:554/* If join requested, exit immediately without broadcast */:exit_threaddoes broadcast in the JOIN_REQUESTED branch, and that broadcast is required to wakeml_single_closewaiting oninvoking. Reword to e.g./* Join requested: release data and broadcast at exit_thread */. - [Low] Comment style / lost invariant —
:562/*Reset state and notify */is missing the space after/*; and the original/* Do not set IDLE if JOIN_REQUESTED */at the exit label was replaced by/* Cleanup resources on exit */, dropping a non-obvious invariant thatml_single_close/invoke_internalrely on. Please restore it alongside the new comment. - [Low] Stale Android CI failure — the PR's Android Build Test (2025-12-26) failed on all four ABIs; logs have expired and the diff is platform-independent control flow, so this is almost certainly transient. Please rebase onto current
mainand re-run CI before merging.
No back-door or suspicious behavior found.
Refactor invoke_thread to remove goto statements and simplify control flow