Skip to content

fix: abort async operations #3152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2025
Merged

fix: abort async operations #3152

merged 2 commits into from
May 28, 2025

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented May 27, 2025

Some async operations do not accept an abort signal. This means calling code can fail to abort a long-running operation if the abort signal fires it's "abort" event while execution is suspended.

For example in the following code the signal can fire while the directory is being created and this code would be none the wiser:

import fs from 'node:fs/promises'

async function doSomthing (signal) {
  await fs.mkdir()
}

Instead we need to check if the signal is aborted before starting the async operation (to prevent doing needless work), and after, in case it aborted while the onward call was happening.

import fs from 'node:fs/promises'

async function doSomthing (signal) {
  signal.throwIfAborted()
  await fs.mkdir()
  signal.throwIfAborted()
}

This still waits for the operation to complete before checking the signal to see if it has been aborted. Instead we can use raceSignal to throw immediately, though the operation will continue in the background.

import fs from 'node:fs/promises'
import { raceSignal } from 'race-signal'

async function doSomthing (signal) {
  signal.throwIfAborted()
  await raceSignal(fs.mkdir(), signal)
}

Synchronous operations have a similar problem when they are able to return optional promises, since the result may be awaited on. In this case we still need to check if the signal was aborted, though we only need to do it once.

interface MyOp<T> {
  (signal: AbortSignal): Promise<T> | T
}

const fn: MyOp = (signal) => {
  signal.throwIfAborted()
  return 'hello'
}

// awaiting this shifts the continuation into the microtask queue
await fn(AbortSignal.timeout(1_000))

// could use optional await but `fn` cannot tell if it's being awaited on or not
// so it always has to check the signal
const p = fn(AbortSignal.timeout(1_000))

if (p.then) {
  await p
}

This PR applies the above patterns to @libp2p/crypto keys, @libp2p/peer-store operations and @libp2p/kad-dht to ensure the .provide and .findProviders operations can be successfully aborted.

Some async operations do not accept an abort signal. This means
calling code can fail to abort a long-running operation if the
abort signal fires it's "abort" event while execution is suspended.

For example in the following code the signal can fire while the
directory is being created and this code would be none the wiser:

```js
import fs from 'node:fs/promises'

async function doSomthing (signal) {
  await fs.mkdir()
}
```

Instead we need to check if the signal is aborted before starting
the async operation (to prevent doing needless work), and after, in
case it aborted while the onward call was happening.

```js
import fs from 'node:fs/promises'

async function doSomthing (signal) {
  signal.throwIfAborted()
  await fs.mkdir()
  signal.throwIfAborted()
}
```

This still waits for the operation to complete before checking the
signal to see if it has been aborted.  Instead we can use `raceSignal`
to throw immediately, though the operation will continue in the
background.

```js
import fs from 'node:fs/promises'
import { raceSignal } from 'race-signal'

async function doSomthing (signal) {
  signal.throwIfAborted()
  await raceSignal(fs.mkdir(), signal)
}
```
@achingbrain achingbrain requested a review from a team as a code owner May 27, 2025 17:57
@achingbrain achingbrain merged commit 8efb065 into main May 28, 2025
33 checks passed
@achingbrain achingbrain deleted the fix/abort-async-operations branch May 28, 2025 16:53
@achingbrain achingbrain mentioned this pull request May 24, 2025
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