Skip to content

[single] Refactor invoke_thread to reduce goto statements - #663

Open
songgot wants to merge 1 commit into
nnstreamer:mainfrom
songgot:remove_goto
Open

[single] Refactor invoke_thread to reduce goto statements#663
songgot wants to merge 1 commit into
nnstreamer:mainfrom
songgot:remove_goto

Conversation

@songgot

@songgot songgot commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

Refactor invoke_thread to remove goto statements and simplify control flow

@songgot songgot changed the title [single] Refactor invoke_thread to remove goto statements [single] Refactor invoke_thread to reduce goto statements Nov 18, 2025
Comment thread c/src/ml-api-inference-single.c
Refactor invoke_thread to reduce goto statements and simplify control flow

Signed-off-by: hyunil park <hyunil46.park@samsung.com>

@myungjoo-bot myungjoo-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. [Low] Small semantic change not mentionedc/src/ml-api-inference-single.c:545: single_h->status = status is now also stored when state == JOIN_REQUESTED after __invoke. Harmless (a still-waiting _ml_single_invoke_internal gets 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 the if (single_h->state == RUNNING)) or mention the change in the commit message.
  2. [Low] Inaccurate comment:554 /* If join requested, exit immediately without broadcast */: exit_thread does broadcast in the JOIN_REQUESTED branch, and that broadcast is required to wake ml_single_close waiting on invoking. Reword to e.g. /* Join requested: release data and broadcast at exit_thread */.
  3. [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 that ml_single_close / invoke_internal rely on. Please restore it alongside the new comment.
  4. [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 main and re-run CI before merging.

No back-door or suspicious behavior found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants