Skip to content

Add exception handling in task_worker_loop to prevent worker thread termination#7658

Merged
achamayou merged 14 commits intomainfrom
copilot/add-try-catch-task-do-task
Mar 6, 2026
Merged

Add exception handling in task_worker_loop to prevent worker thread termination#7658
achamayou merged 14 commits intomainfrom
copilot/add-try-catch-task-do-task

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 10, 2026

Task: Add try-catch block around do_task() in worker.h

  • Add necessary include for logging (ds/internal_logger.h)
  • Add try-catch block around task->do_task() call
  • Use task->get_name() for descriptive logging in catch handlers
  • Handle both std::exception and unknown exceptions
  • Create test to verify exception handling
  • Build and verify the changes work correctly
  • Address review feedback: add <exception> include to worker.h
  • Fix test synchronization with bounded timeout (1 second max wait)
  • Format all changes with clang-format
  • Fix clang-tidy errors in worker.cpp:
    • Move current_throw_trace, demangle_symbol, and format_stacktrace to anonymous namespace (misc-use-anonymous-namespace)
    • Use RAII (std::unique_ptr with custom deleters) instead of manual free() (cppcoreguidelines-no-malloc, cppcoreguidelines-owning-memory)
    • Add NOLINTNEXTLINE for unavoidable free() in deleters and multi-level pointer conversion (bugprone-multi-level-implicit-pointer-conversion)
    • Use auto for real_cxa_throw initialization (modernize-use-auto)
    • Construct backtrace_symbols result directly in unique_ptr for safer RAII
Original prompt

Apply the suggestion from this comment, within the task system itself:
#7630 (comment)

Add a try catch block around the call to do_task() in src/tasks/worker.h to prevent exceptions escaping further. Use task->get_name() for descriptive logging.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: eddyashton <6000239+eddyashton@users.noreply.github.com>
Copilot AI changed the title [WIP] Add try catch block around do_task call in worker.h Add exception handling in task_worker_loop to prevent worker thread termination Feb 10, 2026
Copilot AI requested a review from eddyashton February 10, 2026 14:24
@eddyashton eddyashton marked this pull request as ready for review February 10, 2026 14:26
@eddyashton eddyashton requested a review from a team as a code owner February 10, 2026 14:26
Copilot AI review requested due to automatic review settings February 10, 2026 14:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the task system’s robustness by preventing exceptions thrown from BaseTask::do_task() from escaping the worker loop and terminating worker threads, and adds a regression test to ensure subsequent tasks still run after failures.

Changes:

  • Add worker-level try/catch around task->do_task() with logging for std::exception and unknown exceptions.
  • Add a doctest verifying the worker continues processing after tasks throw.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/tasks/worker.h Adds exception boundary + failure logging around task execution to keep worker threads alive.
src/tasks/test/basic_tasks.cpp Adds a regression test covering continued execution after exceptions.

Comment thread src/tasks/test/basic_tasks.cpp Outdated
Comment thread src/tasks/worker.h
Comment thread src/tasks/test/basic_tasks.cpp Outdated
@eddyashton
Copy link
Copy Markdown
Member

@copilot apply changes based on the comments in this thread, and format all changes with clang-format.

…nization, format code

Co-authored-by: eddyashton <6000239+eddyashton@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Feb 10, 2026

@copilot apply changes based on the comments in this thread, and format all changes with clang-format.

Applied all review feedback and formatted with clang-format (23843eb). All 13 tests pass with 265/265 assertions.

Comment thread src/tasks/worker.h Outdated
@achamayou
Copy link
Copy Markdown
Member

@copilot fix these:

/usr/bin/cmake -E __run_co_compile --tidy="/usr/bin/clang-tidy;--extra-arg-before=--driver-mode=g++" --source=/__w/CCF/CCF/src/tasks/worker.cpp -- /usr/bin/clang++  -I/__w/CCF/CCF/include -I/__w/CCF/CCF/src -isystem /__w/CCF/CCF/3rdparty/exported -isystem /__w/CCF/CCF/3rdparty/internal -g -std=gnu++20 -fPIC -fcolor-diagnostics -ffile-prefix-map=/__w/CCF/CCF=CCF -fno-omit-frame-pointer -Wall -Wextra -Werror -Wundef -Wpedantic -Wno-unused-parameter -Wno-unused-function -Wshadow -MD -MT CMakeFiles/ccf_tasks.dir/src/tasks/worker.cpp.o -MF CMakeFiles/ccf_tasks.dir/src/tasks/worker.cpp.o.d -o CMakeFiles/ccf_tasks.dir/src/tasks/worker.cpp.o -c /__w/CCF/CCF/src/tasks/worker.cpp
/__w/CCF/CCF/src/tasks/worker.cpp:23:34: error: variable 'current_throw_trace' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace,-warnings-as-errors]
   23 |   static thread_local ThrowTrace current_throw_trace = {};
      |                                  ^
/__w/CCF/CCF/src/tasks/worker.cpp:25:22: error: function 'demangle_symbol' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace,-warnings-as-errors]
   25 |   static std::string demangle_symbol(const char* raw)
      |                      ^
/__w/CCF/CCF/src/tasks/worker.cpp:51:9: error: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc,-warnings-as-errors]
   51 |         free(demangled);
      |         ^~~~~~~~~~~~~~~
/__w/CCF/CCF/src/tasks/worker.cpp:51:9: error: calling legacy resource function without passing a 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
   51 |         free(demangled);
      |         ^~~~~~~~~~~~~~~
/__w/CCF/CCF/src/tasks/worker.cpp:63:22: error: function 'format_stacktrace' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace,-warnings-as-errors]
   63 |   static std::string format_stacktrace(void** frames, int num_frames)
      |                      ^
/__w/CCF/CCF/src/tasks/worker.cpp:71:5: error: do not manage memory manually; use RAII [cppcoreguidelines-no-malloc,-warnings-as-errors]
   71 |     free(symbols);
      |     ^~~~~~~~~~~~~
/__w/CCF/CCF/src/tasks/worker.cpp:71:5: error: calling legacy resource function without passing a 'gsl::owner<>' [cppcoreguidelines-owning-memory,-warnings-as-errors]
   71 |     free(symbols);
      |     ^~~~~~~~~~~~~
/__w/CCF/CCF/src/tasks/worker.cpp:71:10: error: multilevel pointer conversion from 'char **' to 'void *', please use explicit cast [bugprone-multi-level-implicit-pointer-conversion,-warnings-as-errors]
   71 |     free(symbols);
      |          ^
/__w/CCF/CCF/src/tasks/worker.cpp:113:12: error: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto,-warnings-as-errors]
  113 |     static CxaThrowFn real_cxa_throw =
      |            ^~~~~~~~~~
      |            auto

…nd auto

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 6, 2026

@copilot fix these:

...

Fixed all clang-tidy errors in 8b71751:

  • misc-use-anonymous-namespace: moved current_throw_trace, demangle_symbol, and format_stacktrace into anonymous namespace
  • cppcoreguidelines-no-malloc / cppcoreguidelines-owning-memory: replaced manual free(demangled) with std::unique_ptr<char, FreeDeleter> RAII wrapper; added FreePtrArrayDeleter for backtrace_symbols result
  • bugprone-multi-level-implicit-pointer-conversion: NOLINTNEXTLINE on the unavoidable free(char**) inside the deleter
  • modernize-use-auto: changed static CxaThrowFn to static auto

Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copilot AI requested a review from achamayou March 6, 2026 07:41
Comment thread src/tasks/worker.cpp Outdated
Comment thread src/tasks/worker.cpp Outdated
@achamayou achamayou merged commit 4e02b8b into main Mar 6, 2026
17 checks passed
@achamayou achamayou deleted the copilot/add-try-catch-task-do-task branch March 6, 2026 11:27
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.

4 participants