Skip to content

Refuse processing thread pushes after stop() and after a task threw - #285

Merged
5cript merged 9 commits into
mainfrom
fix/processing-thread-refused-tasks
Oct 28, 2026
Merged

5cript merged 9 commits into
mainfrom
fix/processing-thread-refused-tasks

Conversation

@5cript

@5cript 5cript commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Part of #260.

ProcessingThread no longer accepts tasks after stop() (until the next start()) or after a task threw, and a refused or dropped promise task completes its future with an error value instead of a broken promise. Before, such tasks ran later in the destructor on the destroying thread, or never ran while their callers waited on them.

  • The stop flag is set and checked under the task mutex and stop() drains outside it, so a push racing stop() is either run by the drain or refused. A promise task refused during the drain completes once the drain reached it, so it still works as a barrier for the tasks queued before it.
  • pushPromiseTask takes an optional unfinished result. UnfinishedTaskResult provides it for void (nothing), bool (false) and std::expected of SftpError (TaskPushFailed) or OperationError (ChannelClosed). A finalized strand answers the same way instead of with a runtime_error.
  • After a task threw, queued tasks are dropped and their futures complete with that error. workQueue then fails the operations that need the strand, one per cycle, instead of blocking in get().
  • Strand final tasks are cleanup tasks: they still run when the loop fails, and after stop() on the calling thread, which acts as the processing thread meanwhile, so SftpSession::close and Channel::close keep removing themselves. The drain and these caller-side runs contain task exceptions like the loop does.
  • removePermanentTask from another thread during a permanent cycle returns whether it removed the task, not whether the deferred removal ran.
  • Session::createPtyChannel and createSftpSession go through pushPromiseTask, and FileStream::readAll and a chunked FileStream::write complete with TaskPushFailed when a read or chunk cannot run (FileNull on a closed file).

…art()

- The flag is set and checked under taskMutex_, so a push that passed the check no longer lands after the drain
- stop() runs the drained tasks outside the lock, pushes meanwhile are refused instead of waiting for the drain
…en promise

- UnfinishedTaskResult makes it: nothing for void, false for bool, the error of a std::expected (SftpError and
  OperationError define one); other results pass their own
- A finalized strand completes its promise tasks the same way instead of with a runtime_error
- awaitCycle reads the value, workQueue fails the first strand operation when its batch did not run
…l queued

- Queued tasks are dropped there, so their promise tasks complete as unfinished instead of waiting for stop()
- Strand final tasks are cleanup tasks: they still run when the loop fails, and after stop() on the calling thread,
  so SFTP sessions and channels close and leave their owner
- stop() drains as the processing thread; cleanup tasks outside the loop run one at a time under cleanupMutex_
- From another thread during a permanent cycle, removePermanentTask returned whether the deferred removal ran, so a
  second removal reported success (CanSafelyDoubleDeletePermanentTask flaked, 9 in 50000 TSan runs)
- A refused or dropped push completes the future with an error instead of a broken promise
…ot run

- A refused, unpushed or dropped chunk completes the future with TaskPushFailed, a closed file with FileNull
… barriers

- A loop that throws while stop() drains leaves the queue and intake to the drain, which resets the thread id and
  closes intake together, so no cleanup task runs on its caller beside stop()
- The drain and cleanup tasks run on a caller contain task exceptions like the loop does
- A promise task pushed while stop() drains is queued refused and completes when the drain reaches it, so
  Operation::waitForStrandTasks still waits for the tasks queued before it
- A refused, unpushed or dropped read completes the future with TaskPushFailed instead of a broken promise
@5cript
5cript enabled auto-merge (rebase) October 28, 2026 23:07
@5cript
5cript merged commit e7b4e7d into main Oct 28, 2026
3 checks passed
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.

1 participant