jsonnull/electron-trpc

Risk for memory leaks with subscriptions on page reload

NordlingDev opened this issue · 4 comments

Sometimes during development it's necessary to fully reload the page. I created this little subscription procedure to see whether there is a risk for memory leaks. And yes, there is. Check the code below:

let count = 0;

export const testSubscription = publicProcedure.subscription(async () => {
  return observable<string>(() => {
    count ++;
    console.log(count);

    return () => {
      count --;
    };
  });
});

Whenever this procedure is being subscribed to, count will increment or decrement to keep track of amount of active subscriptions. However, if you do a page reload, there is no cleanup process being made and count keeps increasing.

I fixed this with a simple workaround inside the app itself (React app) by having a beforeunload listener on the window to unmount the app before the page actually reloads. See below:

import { StrictMode, useEffect } from "react";
import * as ReactDOM from "react-dom/client";

import { Root } from "./app";

const root = ReactDOM.createRoot(document.getElementById("root") as HTMLElement);

window.addEventListener("beforeunload", () => {
  root.unmount();
});

root.render(
  <StrictMode>
    <Root />
  </StrictMode>,
);

Now, I'm not sure if this is the prettiest way to do it and neither am I sure it covers all potential cases. So is this is something worth investigating?

Hey, so I looked into this and I don't think there's an actual leak happening here.

When we create new subscriptions for a given window, we'll actually overwrite existing subscriptions for that window when the new ones are created. In some ways the behavior is the same as tRPC over browser+server, but the key difference is that a tRPC server can catch broken websocket connections and perform cleanup. I think electron-trpc can do this as well, but we don't currently.

@jsonnull Yeah there is no leak actually happening. I was just saying there is a risk for it if devs aren't aware that no cleanup is being made to subscriptions when "connections" are lost (due to page reload in this case).

Marking this as completed as of #165, which is out in 0.6.0.