Skip to content

SwiftyThreadPool.cancel() leaves the pool permanently unusable, and restart() silently does nothing #6

Description

@haoking

Summary

SwiftyThreadPool.cancel() sets the internal queue to nil. Every subsequent operation on that pool then fails silently:

  • restart() is currentQueue?.isSuspended = false — optional-chaining on nil, so it is a permanent no-op. The pool cannot be revived.
  • add(_:withIdentifier:) begins guard let queue = currentQueue else { return } — work submitted afterwards is silently discarded, with no error, no return value, and no log.

Nothing reports that the pool is dead. A caller holding a reference sees a normal-looking object that accepts work and drops it.

Why this surfaced now

2.1 made Promise observe operation cancellation, so a cancelled chain now fails with PromiseError.cancelled instead of silently completing with a stale value. While documenting exactly when Promise.value() can still fail to return, this turned out to be one of the cases — and the least recoverable one.

Sources/SwiftyPromise.swift's value() doc comment enumerates three remaining hang paths. Two of them trace back to this issue:

  • A pool cancel()ed before a chain is dispatched: add() drops the operation, so it never enters any queue, no completionBlock ever fires, and the cancellation-detection path added in 2.1 cannot catch it. try await promise.value() hangs forever.
  • A pool cancel()ed while suspended: the already-queued operations are cancelled but the queue never finishes them (a suspended OperationQueue finishes nothing, including cancelled operations), and because cancel() nils the queue, restart() can no longer revive it. Unrecoverable.

Reproduction

let pool = SwiftyThreadPool()
pool.cancel()
pool.restart()                 // no-op: currentQueue is nil
print(pool.count)              // 0 — indistinguishable from "idle"
pool.add(BlockOperation { print("never runs") })   // silently dropped

Suggested direction

Several options, not mutually exclusive:

  1. Have cancel() cancel and drain without discarding the queue, so the pool stays usable and restart() keeps meaning something. This is the smallest change and matches what most callers likely expect from a method named cancel.
  2. If a terminal state is intended, make it explicit and observable — an isCancelled/isTerminated property, and have add() signal rejection instead of returning Void silently.
  3. At minimum, document on cancel() that it is terminal and that restart() will not revive the pool.

Option 1 also closes two of the three hang paths documented on Promise.value().

Related

Discovered while implementing #3. Not fixed in 2.1 because it changes SwiftyThreadPool's public lifecycle semantics rather than Promise's cancellation observation, which was that release's scope.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions