Skip to content
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

Prevent accumulation of subscription notifications when the last AsyncIterator falls out of scope #3090

Open
steveluscher opened this issue Aug 9, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@steveluscher
Copy link
Collaborator

steveluscher commented Aug 9, 2024

Overview

There's something about the chain of AsyncGenerators in the subscriptions transport that results in orphaned AsyncIterators stuffing messages into queuedMessages all day long until you run out of memory.

  1. If you abort the subscription (unsubscribe) these go away. Good.
  2. If, however, you're holding on to an AsyncIterator and you stop iterating over it, an inner iterator won't learn of that fact; it will keep pumping notifications into queuedMessages forever.

Steps to reproduce

Instrument around here where messages are added to queuedMessages and watch as the number of messages destined for the AsyncIterator in (A) grows forever.

import { createSolanaRpcSubscriptions_UNSTABLE } from '@solana/web3.js';

const rpcSubscriptions = createSolanaRpcSubscriptions_UNSTABLE(
    'wss://api.devnet.solana.com',
);

const abortSignal = AbortSignal.timeout(5000);

// Imagine this code lives in one part of your app (A).
(async () => {
    const slotNotifications = await rpcSubscriptions
        .slotsUpdatesNotifications()
        .subscribe({ abortSignal });

    console.log('>>>>> (A) start caring about slot notifications');
    for await (const notification of slotNotifications) {
        if (notification.type === 'firstShredReceived') {
            // Stop iterating after receiving a particular notification.
            break;
        }
    }
    console.log('>>>>> (A) stop caring about slot notifications');
    // The `AsyncIterator` created by `of slotNotifications` has fallen
    // out of scope and should be eligible for garbage collection now.
})();

// Imagine this code lives in a different part of your app, (B).
(async () => {
    const slotNotifications = await rpcSubscriptions
        .slotsUpdatesNotifications()
        .subscribe({ abortSignal });

    console.log('>>>>> (B) start caring about slot notifications');
    for await (const _notification of slotNotifications) {
        // Use every notification. Never stop.
    }
    console.log('>>>>> (B) stop caring about slot notifications');
})();

Description of bug

Right now the AsyncIterable returned by subscribe() offers certain guarantees about message delivery that we might have to reevaluate. In particular, it guarantees that if there's a delay between having processed a value and the time you ask the AsyncIterator for the next value, any subscription notifications received during that time will be queued and delivered to you all at once. If you never ask for the next value and you never abort the subscription (unsubscribe) then this queue grows forever.

We might need to eliminate this guarantee. Alternatively, maybe this is one of those rare use cases for FinalizationRegistry, if only the thing that's doing the queueing can know when the thing consuming the messages has fallen out of scope.

@steveluscher steveluscher added the bug Something isn't working label Aug 9, 2024
@steveluscher steveluscher changed the title Prevent accumulation of notifications when the last AsyncIterable falls out of scope Prevent accumulation of notifications when the last AsyncIterator falls out of scope Aug 9, 2024
@steveluscher steveluscher changed the title Prevent accumulation of notifications when the last AsyncIterator falls out of scope Prevent accumulation of subscription notifications when the last AsyncIterator falls out of scope Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant