Skip to content

bugfix/guard_page_and_ex_handle#13

Merged
Melnytskyi merged 8 commits into
mainfrom
copilot/complete-todos-in-scheduler-and-light-stack-again
May 18, 2026
Merged

bugfix/guard_page_and_ex_handle#13
Melnytskyi merged 8 commits into
mainfrom
copilot/complete-todos-in-scheduler-and-light-stack-again

Conversation

Copilot AI commented May 18, 2026

Copy link
Copy Markdown
Contributor
  • Review new PR comment and identify actionable review threads to address
  • Inspect current code and run existing build/tests to establish baseline
  • Implement fixes for scheduler abort_tasks behavior and executing_tasks accounting
  • Implement guard-page-safe stack clearing on Linux/Windows reuse paths
  • Fix CMake source-dir include path and validate FAST_TASK_GUARD_PAGE_COUNT input
  • Add/adjust tests for abort_tasks=true queued-task shutdown behavior
  • Run targeted tests, then final validation (including parallel validation)
  • Fix stack allocation to always reserve size + guard_page_size (new comment)
  • Validate and push fix

Copilot AI and others added 5 commits May 18, 2026 00:47
- Add fast_task::stack_overflow exception to include/exceptions.hpp
- Implement Linux x86_64 SIGSEGV handler in light_stack.cpp:
  * Install per-thread alternate signal stack + process-wide sigaction
  * On guard-page fault: mprotect to RW, set up return address from [RBP+8]
    so the DWARF unwind chain is intact, redirect RIP to __stack_overflow_raise
  * Restore PROT_NONE on guard page when recycling stacks (unlimited/limited_buffer)
- Add CMake fallback to valgrind stub headers when system valgrind not installed
- Add tests/stackfull/test_stackfull_guard.cpp (4 tests, all pass)

Agent-Logs-Url: https://github.com/Melnytskyi/fast_task/sessions/7235f5d5-bf5b-4d9b-860b-d2d013dbcf4e

Co-authored-by: Melnytskyi <[email protected]>
- Check mprotect() return value in signal handler; fall back to old handler on failure
- Validate RBP is within valid (non-guard) stack bounds before dereferencing [RBP+8]
- Check sigaltstack() return value; free alt-stack memory on failure
- Discard stack in unlimited_buffer/limited_buffer if mprotect(PROT_NONE) fails

Agent-Logs-Url: https://github.com/Melnytskyi/fast_task/sessions/7235f5d5-bf5b-4d9b-860b-d2d013dbcf4e

Co-authored-by: Melnytskyi <[email protected]>
…ow recovery

- Add FAST_TASK_GUARD_PAGE_COUNT cmake option (default 1); passes FT_GUARD_PAGE_COUNT
  to the library so guard page size is configurable at build time; 0 disables mprotect
- Remove stack_overflow exception type from exceptions.hpp
- Linux light_stack: remove SIGSEGV handler machinery (handler, alt-stack, sigaction);
  make __install_signal_handler_mem a no-op; drive guard_page_size from FT_GUARD_PAGE_COUNT;
  guard mprotect(PROT_NONE) in create_stack with #if FT_GUARD_PAGE_COUNT > 0;
  remove mprotect restoration from unlimited_buffer / limited_buffer
- Windows light_stack: drive guard_page_size from FT_GUARD_PAGE_COUNT; guard
  PAGE_GUARD commit with #if FT_GUARD_PAGE_COUNT > 0
- test_stackfull_guard: remove stack-overflow tests; keep 2 exception-callback tests

Agent-Logs-Url: https://github.com/Melnytskyi/fast_task/sessions/463eb9a4-b8be-4088-becb-33ec6fdfbd0e

Co-authored-by: Melnytskyi <[email protected]>
@Melnytskyi Melnytskyi changed the title Guard pages as fast-fail with configurable count; remove overflow-recovery machinery bugfix/guard_page_and_ex_handle May 18, 2026
@Melnytskyi Melnytskyi requested a review from Copilot May 18, 2026 15:29

Copilot AI 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.

Pull request overview

This PR simplifies stack guard handling by moving away from stack-overflow recovery and adds configurable guard-page sizing, Valgrind header fallbacks, exception-callback coverage, and an abort option for closing bind-only executors.

Changes:

  • Adds FAST_TASK_GUARD_PAGE_COUNT and applies guard-page sizing on Linux/Windows stacks.
  • Adds bundled minimal Valgrind header stubs for builds without system Valgrind headers.
  • Adds exception callback tests and extends bind-only executor close API with abort_tasks.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
CMakeLists.txt Adds guard-page count definition and Valgrind stub include fallback.
include/task/scheduler.hpp Extends close_bind_only_executor with abort_tasks.
src/tasks/_internal.hpp Stores binded executor abort-on-close state.
src/tasks/classes/task/scheduler.cpp Propagates abort-on-close state while closing bind-only executors.
src/tasks/scheduler.cpp Handles in-place exception callbacks and aborts queued binded tasks on close.
src/tasks/util/light_stack.cpp Applies configurable guard-page behavior and removes Linux signal-handler recovery.
tests/stackfull/CMakeLists.txt Registers the new stackfull guard test target.
tests/stackfull/test_stackfull_guard.cpp Adds portable exception-callback and RAII unwinding tests.
third_party/valgrind-stubs/valgrind/memcheck.h Adds minimal Memcheck no-op macros.
third_party/valgrind-stubs/valgrind/valgrind.h Adds minimal Valgrind no-op macros.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/tasks/classes/task/scheduler.cpp
Comment thread src/tasks/scheduler.cpp Outdated
Comment thread src/tasks/util/light_stack.cpp
Comment thread src/tasks/util/light_stack.cpp
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt
Comment thread include/task/scheduler.hpp
@Melnytskyi

Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread

Copilot AI commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Implemented the requested review-thread fixes in commits c2a5790 and b4c6989: abort-on-close now correctly handles queued bind-worker tasks (without rescheduling when aborting), aborted tasks now mark completed and decrement glob.executing_tasks, guard-page stack clearing skips protected ranges safely on Linux/Windows, FAST_TASK_GUARD_PAGE_COUNT is validated as a non-negative integer, Valgrind stub include uses CMAKE_CURRENT_SOURCE_DIR, and a bind-executor test now covers close_bind_only_executor(id, true) with queued tasks plus shutdown/await behavior.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/apt/methods/https /usr/lib/apt/methods/https -I /home/REDACTED/work/fast_task/fast_task/build/_deps/boost-src/libs/atomic/include -I (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from Melnytskyi May 18, 2026 15:51
@Melnytskyi

Copy link
Copy Markdown
Owner

@copilot change the stack allocation to allocate size + guard_pages_size instead of just size to keep the behavior as expected regardless of the library configuration (big guard page setting could set whole stack as guard page and the coroutine couldn't start)

Copilot AI commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@copilot change the stack allocation to allocate size + guard_pages_size instead of just size to keep the behavior as expected regardless of the library configuration (big guard page...

Fixed in commit 3be6541. Both the Windows and Linux allocate() functions now compute size__ = ((size + guard_page_size + page_size - 1) / page_size) * page_size — exactly size + guard_page_size (page-aligned) — removing the old (pages + 1) * page_size formula that added a spurious extra page. The usable portion of every allocated stack is now exactly the requested size regardless of FT_GUARD_PAGE_COUNT.

@Melnytskyi Melnytskyi marked this pull request as ready for review May 18, 2026 16:53
@Melnytskyi Melnytskyi requested a review from Copilot May 18, 2026 16:53

Copilot AI 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.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Comment thread src/tasks/classes/task/scheduler.cpp
Comment thread tests/scheduler/test_bind_executor.cpp
Comment thread CMakeLists.txt
Comment thread src/tasks/util/light_stack.cpp
Comment thread src/tasks/util/light_stack.cpp
Comment thread src/tasks/scheduler.cpp
Comment thread include/task/scheduler.hpp
@Melnytskyi Melnytskyi merged commit fcde43f into main May 18, 2026
1 check passed
@Melnytskyi Melnytskyi deleted the copilot/complete-todos-in-scheduler-and-light-stack-again branch May 18, 2026 17:07
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