GoTrueClient Memory Leak

[ghost]

TLDR

This line is causing a memory leak:
https://github.com/supabase/gotrue-js/blob/15c7c8258b2d42d3378be4f7738c728a07523579/src/GoTrueClient.ts#L2040

setInterval will persist even if the object calling it (in this case, GoTrueClient) has been dereferenced. This prevents garbage collection (as setInterval is still holding a reference, even if the user has lost all references) and leads to the memory leak.

A solution seems to have been attempted, but it is not working in (at the very least) React Native, where I encountered this issue:
https://github.com/supabase/gotrue-js/blob/15c7c8258b2d42d3378be4f7738c728a07523579/src/GoTrueClient.ts#L2043

See Also

• the issue confirmed: https://stackoverflow.com/questions/41429030/does-setinterval-inside-an-object-prevent-garbage-collection-when-the-object-is
• potential solution: https://github.com/nbarrow-inspire-labs/bug-demo/blob/main/src/IntervalTimer.ts
  • blocked in React Native: Hermes WeakRef Support in React Native facebook/react-native#42742
• docs: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef

History

I originally opened this here (supabase/supabase-js#983) but moved once I discovered the setInterval issue.

Bug report

• I confirm this is a bug with Supabase, not with my own application.
• I confirm I have searched the Docs, GitHub Discussions, and Discord.

Describe the bug

Calling createClient results in a GoTrueClient memory leak on React Native. It may be the entire SupabaseClient, but I only tested with GoTrueClient after noticing massive memory issues. See demo repo and video below.

EDIT!: I have tracked the issue down to this line of code:
https://github.com/supabase/gotrue-js/blob/15c7c8258b2d42d3378be4f7738c728a07523579/src/GoTrueClient.ts#L2040

It looks like setInterval persists even after an object is dereferenced, avoiding garbage collection. Can we transfer this to @supabase/gotrue-js?

To Reproduce

I created a demo repo here. This is an extremely simplistic example; in our production app, we are using a router to navigate and noticed that (navigating between pages) we had memory leaks and supabase clients persisting, even though the page was no longer even being rendered and the supabase client should have been destroyed. I could not post our production app publicly, so I made this simple demo to show how even though I no longer have any reference to the client in my code (since it is created and destroyed after every button press), the auth client still exists and sends ticks in the background.

NOTE: as seen in the video below, after 2 button presses (and about 10 seconds) you can see log messages from two supabase clients (GoTrueClient@0 and GoTrueClient@1). These clients are not referenced anywhere; they were (a) created on a button press, then (b) all reference to them was lost. Garbage collection would usually take care of this (I have let it run for several minutes at a time and still had references; I've ever tried creating hundreds, losing the reference, and all hundreds of them persist for seemingly without end).

Edit: I let the app run, without touching it at all, while writing this bug report (the same one shown in video, uninterrupted) and I was still seeing logs from GoTrueClient@1 (and @0) at 2024-02-28T01:43:36.273Z, which is quite a while after in terms of waiting for some kind of garbage collection to have kicked in (assuming there was not a memory leak, but it clearly seems like there is). Here is a log dump showing the clients persisting for quite some time (untill the process was killed): logs.txt

Expected behavior

Supabase clients should not be persisting when dereferenced; somehow garbage collection is being distorted.

Screenshots

1a6a7d82-4e3e-4979-9ae7-acf2326aba62.mp4

System information

• Version of supabase-js: ^2.39.7

[ghost]

I've been playing with something like this, but it won't work in React Native as there is no support for WeakRef nor the FinalizationRegistry (facebook/react-native#42742):

https://github.com/nbarrow-inspire-labs/bug-demo/blob/main/src/IntervalTimer.ts

[ghost]

I just wanted to bump this and see if there's a potential work-around while ReactNative works on supporting WeakRef, etc? My example is a good starting point but clearly not production ready (until WeakRef finds support).

[ghost]

A pub/sub method might be more desireable:

The static class can act as a publisher, using setInterval once at the static level to emit tick events on a regular pattern. All instantiated instances could then subscribe to the events that the static class publisher is emitting; when the static class emits a tick event, the instances could 'do' the tick function.

This would solve the garbage collection issue, as the subscribers exist independently of the one main (static) publisher; if an instance is due for garbage collection, so too would its event subscriber, allowing instances to be garbage collected while the static class publisher is just always running in the background.

Perhaps a library like https://docs.evt.land/ while supports (perportedly) all JS runtimes.

[ghost]

Is there any updates on this?

[sos0]

Is this only for React Native?

[barrownicholas]

As far as I can tell, it effects non-Deno and non-Node.js environments, because the solution attempted previously only fixes this leak for Deno and Node:
https://github.com/supabase/gotrue-js/blob/15c7c8258b2d42d3378be4f7738c728a07523579/src/GoTrueClient.ts#L2043

[ghost]

For the time being, I think this will work (still testing):

import {useEffect, useState} from "react";
import {createClient} from "@supabase/supabase-js";
import AsyncStorage from "@react-native-async-storage/async-storage";

export const useSupabaseClient = (supabase: {
  url: string;
  keys: {
    anon: string;
  }
}) => {

  // this is important too, to use an initializer function in setState
  const [client] = useState(() => createClient(supabase.url, supabase.keys.anon, {
    auth: {
      storage: AsyncStorage
    }
  }));

  useEffect(() => {
    return () => {
      client.auth.stopAutoRefresh().then(); // this should dump the `Timeout`, eliminating all refs, and allow garbage collection
    }
  }, []);

  return client;
};

The operative line is:

client.auth.stopAutoRefresh().then(); // this should dump the `Timeout`, eliminating all refs, and allow garbage collection

[ghost]

From what I can see, ^ seems to work until Hermes has support for WeakRef... once that happens, I can do a PR for a better fix

[uncvrd]

I'm pretty sure I'm dealing with something similar in Node.JS land. I just migrated my app to Supabase and my memory usage now climbs steadily over time until i get an OOM

Around March 9th, i updated my app to only call supabase.auth.getUser(); when necessary instead of on each request. As you can see the slope of memory usage decreased (but since i still need to call this method for authenticated endpoints, the memory usage still grows...just slower). The steep drops are from app restarts.

I see spikes when there are large batches of supabase.auth.getUser() called. Running a memory heap from a load test shows that getItemAsync() and _acquireLock() are the biggest offenders. I can provide this heap if necessary. Anyone else running in to this problem serverside?

[hf]

Not sure why it's a "memory leak," probably the React Native runtime is a bit bad at tracking this. But, as far as I know, you don't have to use this in React Native. You can call supabase.auth.startAutoRefresh() and supabase.auth.stopAutoRefresh().

Take a look at these docs: https://supabase.com/docs/reference/javascript/auth-startautorefresh

[ghost]

@hf the only memory leak will occur in react runtimes that are not (a) Node.js and (b) Deno because of the way setInterval works: if you hold a reference, this is a "strong" reference that will prevent garbage collection. The problem does not exist because, in Node.js, they have a specific dereferencing feature (https://nodejs.org/api/timers.html#timeoutunref) and same with Deno (https://deno.land/[email protected]?s=Deno.unrefTimer) which Supabase already implements automatically:

https://github.com/supabase/gotrue-js/blob/15c7c8258b2d42d3378be4f7738c728a07523579/src/GoTrueClient.ts#L2043

Basically, in non-Node.js and non-Deno runtimes (like hermes in ReactNative) calls to setInterval that reference objects are enough to prevent garbage collection; even if you lose all references to the object, as in this bug report, the setInterval prevents garbage collection as it counts as a reference (even though you've lost all references to the underlying object itself).

If you see my example above (#856 (comment)), my temporary fix was already using the stopAutoRefresh as you mention for non-compatible runetimes (utilizing the callback/return feature when useEffect unmounts).

I also have a solution using the WeakRef api to completely fix this on most runtimes, but React Natives hermes engine is not releasing support for WeakRef until Q2/Q3 this year: facebook/react-native#42742 (comment)

[uncvrd]

@nbarrow-inspire-labs i guess I'm a bit confused ...I'm also dealing with a memory leak but I use Supabase in a Node.JS context so it's still affecting me as well, not just in React Native (etc.). Calling .getUser() causes my memory in a Fastify server to steadily increase over time 🤔

[ghost]

@uncvrd can you try the debug option in your createClient (I believe it is as follows):

  const [client] = useState(() => createClient(supabase.url, supabase.keys.anon, {
    auth: {
      debug: true
    }
  }));

Perhaps the logs will give some insight? Also, if you see the number of clients increasing (each log statement indicates which number client it is) then you might be having the exact same issue, but for a different reason.

[uncvrd]

@nbarrow-inspire-labs oh thank you for leading me to the debug option! Yikes, yes, i ran a load test for 45 seconds and it has created 4000+ clients it seems like

[ghost]

@uncvrd yeah you definitely have a memory leak problem (youre correct that 4267 clients were created in your short testing period)... can I see the load test you ran (if it was a simple test repo) or perhaps can you post snipets of how you are creating your supabase clients?

[uncvrd]

For sure! I'll post snippets for now and if needed, can break out a test repo if needed

I have a simple k6 load test hitting my endpoint. I use tRPC (if you are familiar?) to create my endpoints https://trpc.io/. It allows you to batch multiple requests in to one HTTP request. It might be a bit confusing if you aren't familiar, let me know if you have questions. So in this case I'm hitting several authenticated endpoints in one batched network request.

// load-test.js
export default function () {
    const res = http.get(
        "https://trpc-web.foundry-dev.ac/trpc/authed.orgContact.workspaces,authed.orgContact.permissionsV2,authed.workspaceContact.byOrgContactId,authed.workspace.permissionsV2,orgContact.byId,authed.organization.byId?batch=1&input=%7B%220%22%3A%7B%22json%22%3A%7B%22orgContactId%22%3A%221abc123%22%7D%7D%2C%221%22%3A%7B%22json%22%3A%7B%22orgContactId%22%3A%221abc123%22%7D%7D%2C%222%22%3A%7B%22json%22%3A%7B%22orgContactId%22%3A%221abc123%22%2C%22workspaceId%22%3A1%7D%7D%2C%223%22%3A%7B%22json%22%3A%7B%22orgContactId%22%3A%221abc123%22%2C%22workspaceId%22%3A1%7D%7D%2C%224%22%3A%7B%22json%22%3A%7B%22id%22%3A%221abc123%22%7D%7D%2C%225%22%3A%7B%22json%22%3A%7B%22id%22%3A1%7D%7D%7D",
        {
            cookies: {
                "sb-127-auth-token":  "[REMOVED]",
            },
        }
    );
    check(res, { "status was 200": (r) => r.status == 200 });
}

Once this HTTP request makes it's way to my server, the requests are sent off in parallel to the respective endpoints, the network request first passes through a context function to define the properties I'll have access to within the business logic of the backend endpoint. This reduced context returns an object that has the user (set to null by default) and a function that uses getUser to pull the information from the cookie in the header (using getCookie)

// context.ts
export interface CreateInnerContextOptions {
    headers: IncomingHttpHeaders;
    setCookie?: (name: string, value: string, options?: CookieSerializeOptions) => void;
    getCookie?: (name: string) => string | undefined;
    removeCookie?: (name: string, options: CookieSerializeOptions) => void;
}

export type Context = {
    supabase: SupaClient; // this is a typed SupabaseClient<Database>
    user: User | null;
    getUser: () => Promise<User | null>;
};

export const createContextInner = async (opts?: CreateInnerContextOptions): Promise<Context> => {

    let user: User | null = null;

    return {
        supabase: createServiceRoleClient(),
        user: null,
        // used for public routers so that auth doesn't have to run on each request
        getUser: async () => {
            if (!user) {
                const supabase = createAnonClient(opts);

                const { data } = await supabase.auth.getUser();

                user = data.user;
            }

            return user;
        },
    };
};

The endpoints that started with authed. must pass through a middleware that will populate the user property on the above object. I do that by creating the following middleware:

// trpc.ts
const isAuthed = t.middleware(async ({ next, ctx, meta }) => {
    const user = await ctx.getUser();

    if (!user) {
        throw new TRPCError({ code: "UNAUTHORIZED" });
    }

    return next({
        ctx: {
            user,
        },
    });
});

As you can see, this calls the getUser method from the context object to get the supabase user upon request. If the user exists, it updates the context with the user object from supabase and can be accessed in a API endpoint like this

   // trpc/routes/currentUser.ts
    byCtx: authedProcedure.query(async ({ ctx }) => {
        const { data } = await ctx.supabase.from("User").select("*").eq("id", ctx.user.id).maybeSingle();

        return data;
    }),

I had previously used an old context.ts that called the .getUser method on each request which caused the memory to spike way faster, calling it on demand in the middleware reduced the memory usage:

// OLD BAD WAY

export const createContextInner = async (opts?: CreateInnerContextOptions): Promise<Context> => {
    const supabase = createAnonClient(opts);

    const { data } = await supabase.auth.getUser();

    let user: User | null = data.user;

    return {
        supabase: createServiceRoleClient(),
        user,
    };
};

Does this help a bit? I know it's a lot. I can make a video demo stepping through all of these hoops

[ghost]

@uncvrd given your usecase, it sounds like the ctx object that gets passed along (which you use to call, for example, ctx.supabase.from...) is destroyed after each request is handled?

If so, I would modify all of your createClient calls, to create the actual supabase clients, to apply the step that I use above (calling await supabase.auth.stopAutoRefresh()).

If you are not destroying the ctx after each request, you will still likely want to implement some way of cleaning-up every single supabase instance you make, by calling .auth.stopAutoRefresh at some point. I.e., in React, right before the component/page exists/unmounts, I call .auth.stopAutoRefresh on the supabase client. In your scenario, whenever you are 'done' with a particular supabase instance (created from calling createClient) try calling .auth.stopAutoRefresh and see if that helps.

Note that garbage collection, in my tests, took upto 15-20 seconds, so it might take around that amount of time before you stop seeing logs.

Try this and let me know if that decreases memory usage.

[ghost]

I created a PR that would at least notify users if their platform will experience a memory leak: #867

[uncvrd]

@nbarrow-inspire-labs really appreciate your time and insight. I'm going to try the stopAutoRefresh in a middleware after the request has completed. My first thing I am trying is to use a serverClient and add the user JWT manually within my getUser() method

const token = opts?.getCookie?.(`sb-${env.SUPABASE_PROJECT_ID}-auth-token`);

    if (!!token) {
        const supabase = createServiceRoleClient();

        const { data } = await supabase.auth.getUser(JSON.parse(token)["access_token"]);

        return data.user;
    }

    return null;

Not quite sure why the above would help but just wanted to try setting the auth instead of having supabase try to parse the cookie for me to knock out that hypothesis. I'm attempting to disable a couple properties, first just the persistSession: false

let supabase: SupaClient | undefined;

export function createServiceRoleClient(): SupaClient {
    if (!supabase) {
        supabase = createServerClient<Database>(env.SUPABASE_PROJECT_URL, env.SUPABASE_SERVICE_ROLE_KEY, {
            cookies: {},
            auth: {
                // autoRefreshToken: false,
                persistSession: false,
            },
        });
    }

    return supabase;
}

But I would like to try autoRefreshToken: false afterwards. I saw in documentation they have both of these disabled for a serverside client that I think may help?

https://supabase.com/docs/reference/javascript/auth-admin-getuserbyid

[ghost]

@uncvrd yeah I would definitely call .auth.stopAutoRefresh() on any client you create in your backend. The persistSession I believe just stores session info., but since you reconstruct a new client every time it should also be fine to disable.

[uncvrd]

@nbarrow-inspire-labs so i set autoRefreshToken: false and it's odd that even that doesn't prevent the leak...I'll give stopAutoRefresh() during the next deployment hopefully next week

[ghost]

@uncvrd I havne't tried the autoRefreshToken: false option (and its possible that that setting is being ingored/not handled properly 100% of the time), but I have tried it with stopAutoRefresh and had success, so yeah give that a try and see how it goes.

[uncvrd]

@nbarrow-inspire-labs After inspecting the source code, I guess I don't understand how calling stopAutoRefresh would help in my scenario as the only time this is called is if autoRefreshTicker is set to the setInterval()

auth-js/src/GoTrueClient.ts

Line 2078 in 92fefbd

this.autoRefreshTicker = ticker

which is only set if autoRefreshToken is set to true

auth-js/src/GoTrueClient.ts

Line 2224 in 92fefbd

if (this.autoRefreshToken) {

With this disabled, I don't think this would fix things :/

My next guess was that the supabase client is being initialized with an inmemory storage adapter when persistSession is false

auth-js/src/GoTrueClient.ts

Line 238 in 92fefbd

this.memoryStorage = {}

Where I thought that js object might be slowly getting filled with k/v pairs but when debug: true is set, it shows that "getting" the value from this inmem storage returns null every time so I don't think there's anything in this object either

GoTrueClient@0 (0.0.0) 2024-04-01T07:55:58.296Z #getSession() session from storage null

ugh, I'll keep searching

[uncvrd]

Just wanted to report back but my hunch led me down to a memory issue with unidici, the fetch library used by node in newer versions. There were some mentions of memory issues with undici in node v18. I've updated to v20 on April 2nd and while my memory has increased a little bit initially, it seems to have somewhat stabilized, so maybe that was my issue...