Walkthrough

FiberQueue::Run() now wraps its dequeue/execute loop in a try/catch that fatally logs on std::exception. FiberQueue::blocked_submitters() changed from an instance const method to static. FiberQueueThreadPool::Add() now increments/decrements blocked_submitters_ around the push_ec_ wait.

Changes

FiberQueue reliability and accounting

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• romange/helio#605: Both PRs modify FiberQueue's blocked_submitters_ accounting around the push_ec_ wait path and the blocked_submitters() accessor.

Suggested reviewers: glevkovich, BorysTheDev

❌ Failed checks (1 inconclusive)

• X
• Mastodon
• Reddit
• LinkedIn

• Prevent silent worker-thread termination by catching exceptions in FiberQueue::Run().
• Expose blocked-submitter metric as a static accessor for TLS-backed state.
• Track blocked submitters in FiberQueueThreadPool::Add() when waiting for queue capacity.

graph TD
  A["Producers (callers)"] --> B["FiberQueueThreadPool::Add()"] --> C["FiberQueue (per worker)"] --> D[("Bounded task queue")]
  C --> E["EventCount (pull/push)"]
  B -->|"wait when full"| E -->|"notifyAll"| B
  F["Worker thread"] --> G["FiberQueue::Run() (try/catch)"] --> E
  G --> H["Execute tasklets"]

Loading

The following are alternative approaches to this PR:

1. Catch exceptions per-task instead of per-loop

• ➕ One bad task doesn't necessarily kill the entire worker loop
• ➕ Allows logging task-specific context (if available) and continuing to drain the queue
• ➖ Adds overhead on the hot path (per callback)
• ➖ Can mask systemic failures if tasks repeatedly throw

2. Catch-all (catch (...)) with logging/termination policy

• ➕ Prevents non-std exceptions from escaping the thread entry point
• ➕ Makes failure mode explicit and consistent across exception types
• ➖ Harder to preserve exception details beyond 'unknown exception'
• ➖ Still requires a policy choice: abort vs continue vs quarantine worker

Recommendation: If the intended policy is fail-fast on any task exception, the current loop-level try/catch + LOG(FATAL) is reasonable and avoids silent worker death. Consider broadening to catch (...) as well to avoid termination on non-std exceptions, and (optionally) moving the try/catch inside the per-task invocation if continuing after a single task failure is desirable for this system.

Bug fix (1) +15 / -11

fiberqueue_threadpool.ccWrap FiberQueue::Run loop with fail-fast exception handling +15/-11

Wrap FiberQueue::Run loop with fail-fast exception handling

• Wraps the consumer loop in FiberQueue::Run() with a try/catch for std::exception and logs a fatal error on exception. This makes task exceptions visible and avoids silent termination of the worker thread while draining batches.

util/fibers/fiberqueue_threadpool.cc

Refactor (1) +3 / -1

fiberqueue_threadpool.hMake blocked_submitters accessor static and track pool-side blocking +3/-1

Make blocked_submitters accessor static and track pool-side blocking

• Changes blocked_submitters() to a static accessor (matching the TLS-backed static counter). Also increments/decrements blocked_submitters_ in FiberQueueThreadPool::Add() around the push EventCount wait, so producer blocking is counted even when the pool blocks outside FiberQueue::Add().

util/fibers/fiberqueue_threadpool.h

Description

FiberQueue::Run() wraps internal queue/task execution in a try/catch and converts any
std::exception into a fatal log, which is not limited to third-party interoperability boundaries.
This violates the policy of restricting exceptions to third-party boundaries and translating them
into the project’s preferred error representation.

Code

util/fibers/fiberqueue_threadpool.cc[R41-61]

+  try {
+    while (true) {
+      pull_ec_.await(cb);

-    if (is_closed)
-      break;
+      if (is_closed)
+        break;

-    while (count < kBatchSize && queue_.try_dequeue_sc(batch[count])) {
-      ++count;
-    }
+      while (count < kBatchSize && queue_.try_dequeue_sc(batch[count])) {
+        ++count;
+      }

-    push_ec_.notifyAll();
+      push_ec_.notifyAll();

-    for (unsigned i = 0; i < count; ++i) {
-      batch[i]();
+      for (unsigned i = 0; i < count; ++i) {
+        batch[i]();
+      }
+      count = 0;
    }
-    count = 0;
+  } catch (const std::exception& ex) {
+    LOG(FATAL) << "Exception " << ex.what();
  }

Relevance

⭐ Low

PR #605 removed std::exception catch from FiberQueue::Run(); team moved away from internal
try/catch+LOG(FATAL).

PR-#605
PR-#599

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1535202 restricts try/catch usage to third-party interoperability boundaries.
The changed code adds a broad try/catch (const std::exception&) around the internal worker loop
and task execution in FiberQueue::Run(), without indicating a third-party boundary or translating
to a non-exception error type.

Rule 1535202: Restrict exception usage to third-party interoperability boundaries
util/fibers/fiberqueue_threadpool.cc[41-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FiberQueue::Run()` catches `std::exception` around internal logic, which violates the rule that exceptions should be restricted to third-party interoperability boundaries.

## Issue Context
The current code catches any `std::exception` thrown from `pull_ec_.await(cb)` / task execution (`batch[i]()`), and then calls `LOG(FATAL)`. If exceptions must be handled here, they should be confined to a clearly documented boundary case and translated to the project’s non-exception error handling strategy.

## Fix Focus Areas
- util/fibers/fiberqueue_threadpool.cc[41-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

FiberQueue::Run() only catches std::exception, so if a queued Tasklet throws a non-std exception
type it will bypass the handler and hit std::terminate without the intended LOG(FATAL) message. This
reduces crash diagnosability for failures originating from queued tasks.

Code

util/fibers/fiberqueue_threadpool.cc[R41-61]

+  try {
+    while (true) {
+      pull_ec_.await(cb);

-    if (is_closed)
-      break;
+      if (is_closed)
+        break;

-    while (count < kBatchSize && queue_.try_dequeue_sc(batch[count])) {
-      ++count;
-    }
+      while (count < kBatchSize && queue_.try_dequeue_sc(batch[count])) {
+        ++count;
+      }

-    push_ec_.notifyAll();
+      push_ec_.notifyAll();

-    for (unsigned i = 0; i < count; ++i) {
-      batch[i]();
+      for (unsigned i = 0; i < count; ++i) {
+        batch[i]();
+      }
+      count = 0;
    }
-    count = 0;
+  } catch (const std::exception& ex) {
+    LOG(FATAL) << "Exception " << ex.what();
  }

Relevance

⭐ Low

Given PR #605 removed exception catching entirely in Run(), adding catch(...) for non-std exceptions
is unlikely accepted.

PR-#605

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The worker loop is wrapped in a try/catch that only matches std::exception, while Tasklet is an
unconstrained callable wrapper (void() signature), so non-std exceptions from queued tasks will not
be caught/logged by this change.

util/fibers/fiberqueue_threadpool.cc[41-61]
util/fibers/fiberqueue_threadpool.h[27-36]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`FiberQueue::Run()` catches only `const std::exception&`, so exceptions not derived from `std::exception` will not be logged via the new `LOG(FATAL)` path.

### Issue Context
Tasks executed from `batch[i]()` are stored as `Tasklet` (`fu2::function_base<..., void()>`) and can wrap arbitrary callables, so non-`std::exception` throws are possible.

### Fix Focus Areas
- util/fibers/fiberqueue_threadpool.cc[41-61]

### Suggested fix
Add a follow-up handler:
- `catch (const std::exception& ex) { LOG(FATAL) << ...; }`
- `catch (...) { LOG(FATAL) << "Unknown exception"; }`

Optionally include additional context (worker thread/name) in the log message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools