feat(common): use addEventListener instead of onabort in async utils onAbortPromise

[douglascayers]

Background

Identified a potential bug with overwriting an AbortSignal.onabort property.

Problem

The onabort property is being reassigned by the onAbortPromise function, which if a value had already been set then that logic may now not run because it's been overwritten. This can lead to unexpected behavior.

Changes

• Use the addEventListener method which allows multiple listeners to be registered.
• Replace onAbortPromise with resolveEarlyOnAbort to centralize memory management of the abort event listener with the promise race semantics of simply resolving early on abort

[changeset-bot]

🦋 Changeset detected

Latest commit: 74a9a95

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@powersync/common	Patch
@powersync/node	Patch
@powersync/op-sqlite	Patch
@powersync/react-native	Patch
@powersync/tanstack-react-query	Patch
@powersync/web	Patch
@powersync/diagnostics-app	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[simolus3]

Thanks - I agree with this fix.

(sidenode: I think even an extremely short manual description is better than the one generated by copilot - the main thing I needed to know was that external users might want to set onabort on signals passed to onAbortPromise too. "addressing potential bugs and race conditions" isn't really what's going on here)

[douglascayers]

I think even an extremely short manual description is better than the one generated by copilot

Thank you for the feedback. I'll rewrite the descriptions and skip letting AI write them.

[douglascayers]

@simolus3 I've refactored the approach to also mitigate memory leaks, would appreciate your follow up review at your convenience

[simolus3]

This looks good to me, I just think we can simplify the implementation a bit because we only care about Promise<void>:

export function resolveEarlyOnAbort(promise: Promise<void>, signal: AbortSignal): Promise<void> {
  return new Promise((resolve, reject) => {
    const onAbort = () => {
      resolve();
    };

    const addAbortHandler = () => {
      // Use an event listener to avoid interfering with the onabort
      // property where other code may have registered a handler.
      signal.addEventListener('abort', onAbort);
    };

    const removeAbortHandler = () => {
      // Remove the abort handler to avoid memory leaks.
      signal.removeEventListener('abort', onAbort);
    };

    if (signal.aborted) {
      return onAbort();
    }

    addAbortHandler();
    promise.finally(removeAbortHandler).then(resolve, reject);
  });
}