Ability to explicitly terminate the store

[Sawtaytoes]

New Features

NOTE: This could also be considered a bug report because it's causing a memory leak.

What is the new or updated feature that you are suggesting?

I'm wanting to add a store.terminate function to remove the Redux store entirely, close all listeners, and tell all middleware to run their own terminate functionality including the removal of that store from Redux-Devtools.

Why should this feature be included?

Already talked about a bit here: https://twitter.com/acemarke/status/1296514045789507585?s=20.

We need some way to kill of the Redux store without relying on Garbage Collection. In many cases, especially during development, the Redux store has no way of dying.

Why has no one requested this feature?

Typical use cases of a Redux store with React are it being included on a root element with the Provider component from react-redux. In many other applications that use Redux, there is a single store, and it's never removed. Even in cases where there are multiple stores, the idea is that they stick around forever.

I'm building an application where Redux stores will be created and removed based on the page route. Because of this, I need some way to get rid of Redux.

It's not entirely uncommon to want to load up and kill off Redux stores, especially when you have a component library that uses Redux, but you don't want to have the parent application manage the Redux store. It shouldn't matter what the component library uses internally, but if it does use Redux, it should clean up after itself.

Where does this show up?

There are 2 common practices with Redux which are problematic for this use case. The first is, Redux-Devtools, and the second is something like Redux-Observable or Redux-Saga or any custom middleware with event listeners.

In both cases, even doing store = null doesn't fix the issue. Either the store is still being used by Redux-Devtools or event listeners are still active in middleware with no way of knowing we want garbage collection to remove the store.

Personally, I prefer being explicit in removing the store rather than relying on garbage collection. I understand this could cause problems if you have something call dispatch when the store no longer exists, but good code shouldn't run into this situation. My code knows when it's safe to terminate the Redux store, but there's no way to notify Redux-Devtools or middleware so I keep creating new stores over and over.

Example code

https://codesandbox.io/s/redux-store-memory-leak-lg78o

Many stores created in Redux-Devtools

Redux-Devtools has no clue Redux needs to be terminated, so it listens forever and keeps those stores active in memory:

Multiple middlewares loaded

Two listeners responding to click events because Redux didn't notify middleware the store's been terminated:

What docs changes are needed to explain this?

Add .terminate to the docs on store explaining what this does and how middleware should interact with it. In the case of Redux-Observable, all it needs to do is unsubscribe from the root epic and all listeners will be removed automatically via RxJS.

[timdorr]

First, if you're operating a Redux store without state, that's not Redux; it's just messaging. You should just use a plain Observable in that case. If you need to pass that around your component tree, use a context. And then you can kill off all subscribers by using `complete() to "complete" the observable, allowing them to clean up after themselves.

Regardless, you should be unsubscribing as a part of wherever you're subscribing in the first place. That's just how useEffect works:

useEffect(() => {
  const unsubscribe = store.subscribe()
  return unsubscribe
}, [store])

You always return your cleanup action. So, that will unsubscribe if it gets a new store or on unmount. No need to try and unsubscribe from the top down, just do it from wherever the subscription happens. This leads to more reasonable code, puts everything in the same place, and also makes it much more testable.

Just calling some sort of destroy or terminate method on the store doesn't necessarily mean that subscribers are going to clean up after themselves. So, while you may get rid of the listeners, you've still got other memory bloat sitting around that is likely more substantial.

So, this is probably not a good idea because of the bad situations it encourages. You should do your cleanup locally at the subscription site, not try to broadcast that out from a central store.

[Methuselah96]

@Sawtaytoes There is an issue on the Redux DevTools repo for removing a store from Redux DevTools. Feel free to show your support there by either giving it a thumbs up or making a PR to resolve the issue.

[Sawtaytoes]

@timdorr By your response, it sounds like I wasn't clear in my issue. I've removed that section of text about AngularJS to make the feature request less confusing.

The issue I'm bringing up is affecting an application I have today using the Redux library. createStore gets called multiple times because this set of components gets loaded and unloaded. All of my code is unsubscribing properly, but previous Redux store instances stick around forever because I have active JS event listeners and use Redux-Devtools.

I wrote up a couple examples on CodeSandbox per @markerikson and detailed how, even without .subscribe, this is still an issue. You solution to unsubscribe my subscriptions doesn't make sense because I'm not using .subscribe at all.

My code

The actual code in my application does call .subscribe and will call unsubscribe at the correct time. Thing is, it won't remove the store if I have middleware with active event listeners (detailed in the "listener" code example):

const listenerMiddleware = (store) => {
  const callback = () => {
    store.dispatch({ type: "click" });
  };

  window.addEventListener("click", callback);

  return (next) => (action) => {};
};

This is because Redux relies on garbage collection and has no way of notifying active middleware that it needs to get garbage collected. I'd like a standard way for all Redux middleware to know "hey, it's time to kill off anything preventing garbage collection".

Standard middleware actions

I'm expecting to see a store.onTerminate function that gets called once. Middleware will listen to it and clean up when it's called.

For example, Redux-Observable (the library I'm using for Middleware) would do something like this:

const createEpicMiddleware = (rootEpic$) => {
  // ...

  return (store) => {
    // ...
    store.onTerminate(() => {
      rootEpic$.complete()
    })
    // ...
  }
}

If you look at the "listener" example in the code (https://codesandbox.io/s/redux-store-memory-leak-lg78o), click events still happen even though the Redux store is no longer associated. Why? Because the Redux store has middleware with an active JS event listener. It doesn't know to clean up that listener because Redux hasn't told it "hey, I need to get garbage collected now".

There's absolutely no way for me to know to tell the middleware that it should stop remove it's event listener because, as a consumer of Redux, that's not my job. I'd rather explicitly tell Redux store.terminate() and then every middleware library listens to .onTerminate and shuts down all listeners. Using fromEvent in Redux-Observable, this comes for free; although in my window.addEventListener example, I'll have to call removeEventListener manually. No problem; as long as my code gets a notification that it's time to do this.

---

@Methuselah96 Put a 👍 on it!

[timdorr]

I'd argue what you're writing there isn't a middleware. It's not affecting the operation of the store, it's just creating an external subscription which has nothing to do with the store directly. You can and should implement this with the React lifecycle, as that's what's driving your event listener. The Redux store being created isn't a true lifecycle event, it's a side-effect, so you shouldn't be "watching" it by creating a middleware.

const store = useStore()

useEffect(() => window.addEventListener('click', () => store.dispatch('click')), [store])

You've already got a lifecycle with React, so you should use that. Trying to graft one onto Redux isn't going to work out because it's always going to be at odds with the other, more predominant lifecycle (see react-router-redux for an example of this). If you're writing middleware that doesn't affect the store's operation, you probably don't have an actual middleware and should be looking at the existing APIs to implement what you're building.

[Sawtaytoes]

I think my example of listening to click events was misleading. In my case, I'm listening to an event bus, not click events on body. The example was merely there to detail a situation where a JS event listener will cause the store to stay open in perpetuity. It doesn't matter what we're listening to only that a listener exists.

This will still be a problem with Redux-Observable regardless of if I use JS event listeners or not. Redux-Observable uses RxJS which creates subscribers. Those subscribers will remain in memory until they're unsubscribed, completed, or errored. I need Redux to tell Redux-Observable (or any other middleware) when it's terminated so middleware (including Redux-Devtools) can shut down all operations to allow garbage collection.

If we're allowing custom middleware, it makes sense that Redux should be in charge of telling that middleware when to complete operations.

From my limited knowledge, the work to build this requires adding a store.terminate() and store.onTerminate(callback) function. terminate gets called once to execute all onTerminate callbacks. That's 90% of the work. The rest is figuring out how to disable it from allowing more terminations and ensuring documentation is written to allow middleware authors the ability to be terminate-aware.

Is this something where you're reticent because there's no PR and the work required is more substantial?

[Sawtaytoes]

I can think of one alternate solution to what I'm proposing. It's almost the same thing, but requires no changes to Redux itself except the documentation.

We need a standard action, something like @@TERMINATE, which tells middleware to clean up. Not all middleware listens to Redux actions though, so it's a bit tougher to pull off if you're writing custom middleware, but if we're talking a few middleware libraries like Redux-Saga, Redux-Observable, and Redux-Devtools, then a standard action should work just fine.

It doesn't need to be a standard action, each middleware could use its own, but it's easier to have a single standard method of middleware termination.

At that point, no change needs to be made to Redux's code. I'd personally prefer a standard solution with a note about this in the Redux docs. "To free up memory, middleware can allow for abrupt termination by listening for the @@TERMINATE action." Not as slick as a store.onTerminate, but it works mostly the same.

Because either method is optional, existing middleware won't break. In the @@TERMINATE example, there's no Redux API change so Redux and Redux middleware version conflicts won't be a thing.

Is this the kind of solution you'd prefer?

[markerikson]

I think the biggest question here is around semantics.

What does "terminating a middleware" mean in the first place? How is a middleware supposed to respond to this action or store API call?

Given that middleware are composed into a pipeline, and each middleware has a reference to part of the next middleware, how would you even deconstruct that?

[Sawtaytoes]

How are each middleware composed in a pipeline? Do you mean the next function?

Would they need to be removed in-order from outer layer to inner layer to remove references internal to Redux or is this something where outer middleware needs to remove references to inner middleware?

---

With upcoming Module Federation and already-available Snowpack (utilizing micro frontend architectures), there are plenty of other situations that come up where React code creates and removes Redux stores. ReactDOM even provides a way to unmount a component (including the root component) for micro frontends.

Even if my solution isn't going to solve the problem, this is going to be an issue affecting all unmounted Redux applications going forward. "One store to rule them all" includes a lot of tight-coupling; something that you don't want when dealing with micro frontends. Is there another way to free up memory when using middleware so Redux no longer causes memory leaks?

[markerikson]

If you look at the source of applyMiddleware, it does some magic to pass a reference to the next thing in the chain (either a middleware handler or the real store.dispatch function) to the current middleware entry as its next argument.

Those handlers also are closures thanks to the middleware format, so if I have an object declared in the outermost scope of a middleware (ie, inside the (storeApi) => { } block), the next functions have access to all that.

I don't know if it's even possible to unwind those in any way.

Now, I could imagine that if a middleware sees a @@TERMINATE action, it could do scopeClosedVariable = null or something, but that can't unwind the middleware themselves.

[Sawtaytoes]

I see what you're saying where middleware are all related: https://github.com/reduxjs/redux/blob/master/src/applyMiddleware.ts#L80

If I disconnect the Redux store with store = null, then normally it'd garbage collect because it's been disconnected from the main processing thread. Even with middleware composition, it should still allow garbage collection so long as all references to store on the main thread are removed.

I found this article with a great image of how garbage collection works:

Sadly, this article doesn't explain the problem of active listeners, but I think the only things that prevent garbage collection are:

• setInterval (also any active setTimeout functions).
• addEventListener (specifically window.addEventListener).

Both of these cause problems because they're keeping callbacks in memory. The callbacks themselves aren't the issue so much as those callbacks having references to store.dispatch. Obviously not calling window.removeEventListener will be a memory problem regardless of store.dispatch, but Redux itself won't disappear if it's referenced in a callback when the object containing that callback is still in memory.

In the case of my example code, it's doing:

const callback = () => {
  store.dispatch({
    randomNumber,
    type: "click"
  });
};

window.addEventListener("click", callback);

Because we're adding a listener to window and window hasn't been removed, the callback remains in memory. In this case, store also remains in memory because there's an active reference to it.

To be clear:

1. window still has references in the main thread.
2. There is a callback function pointing at store on window.
3. store remains.

If we can remove the reference to window, it would remove store.

Here's the "listeners" graph starting at 10 listeners and showing how we have 3 more listeners (13 total) after forcing garbage collection when using the window listener middleware:

This is expected. It also means I'm 100% sure store is still in memory.

I used the exact same callback, but this time used a DOM listener instead:

document
.querySelector("#listener")
.addEventListener("click", callback);

When changing the route, #listener gets removed from the DOM. This allows garbage collection to take place because the DOM element isn't being referenced anywhere in the main thread.

We can see that here:

After forcing garbage collection, we're back to 10 listeners (what we started with). While I'm not 100% sure store is out of memory, I'm 100% sure the listeners are gone which means store can garbage collect on its own.

By this data, we should be able to successfully kill-off middleware by simply removing JS event listeners, setInterval, and active setTimeout functions. The fact that middleware functions reference each other shouldn't be a problem so long as all pointers to store no longer exist on the main thread.

Am I understanding your concern or missing something completely?

[markerikson]

At this point, I'm really not sure what actual changes are being asked for, tbh :)

[Sawtaytoes]

This is what I'm proposing:

1. A standard @@TERMINATE action that tells middleware to close any active listeners and callbacks.
   1. This gets dispatched directly from store.dispatch({ type: '@@TERMINATE' }).
   2. Gets dispatched from store.terminate().
2. A store.terminate() function which loops through all store.onTerminate(callback) callbacks in middleware so they know when to close any active listeners and callbacks. This doesn't just affect middleware. Anything like react-redux can also remove references to store if it's still got them somewhere.

Consumers of Redux that have a store need to do store = null to get rid of the reference to it in their own apps. As soon as all references to store are removed, Redux (including its middleware) will be garbage collected.

That's the goal of this whole issue. We need some way of removing Redux from memory using garbage collection. Normally this works fine, but since Redux allows for middleware, we need to also notify middleware that Redux has been terminated; thus, they should also stop any active addEventListener, setInterval, and setTimeout callbacks. Doing so will successfully allow Redux to garbage collect.

[markerikson]

My suggestion would be to first try implementing this as a store enhancer, and see how something like that works out.

[haffmaestro]

@Sawtaytoes I'm very much having this problem as well.

The way the app I work on has been written, there was no concern around just making new stores, often as you say, depending on the path.

I'm in a position where I can try to change this by always seeing if there's another instance of the store, and then conditionally resetting it, but it's a change in pattern for sure, but I can see that not always being the case.

Interested in what you come up with.

[Sawtaytoes]

@haffmaestro I haven't had time to do this kind of stuff on my own, so my current workaround is leaving the Redux store on window and re-referencing that instance when I start a new one. Causes other problems (like stale data), but we've worked around it.

These aren't good solutions, just workarounds until someone (could be me) writes up a solution to solve it and puts up PRs to add that solution to Redux Devtools and Redux-Observable. Others will have their own middleware to adjust.

[haffmaestro]

@Sawtaytoes I've just been referencing the old store via closure which works fine, I think the stale data problem is solvable by having some way of RESET action on the store. Which some redux companion libraries mention on https://github.com/markerikson/redux-ecosystem-links/blob/master/reducers.md

[Sawtaytoes]

@haffmaestro Correct. The RESET action is the only way to clear existing Redux state like that, but it's a pain to manage.

On the other hand, to terminate all processes and middleware on the store, another option is using a TERMINATE action like I was suggesting earlier. I still have to handle this manually, but it should allow unsubscribing from Redux-Observable epics.

On the other hand, Redux Devtools remains open and that's the big issue. There's no easy way of clearing it without modifying that library in particular.

[haffmaestro]

@Sawtaytoes I haven't actually tried going down the RESET action path, this https://github.com/omnidan/redux-recycle makes it seem fairly straightforward. You've already tried an approach like this and found it painful? Are there some gotchas I should be aware of?

But, yeah the Redux Devtools issue is the big thorn in my side. I don't like changing application code because a tool I use depends on it, but I'm not sure what other option I have.

Does seem like you'd need both RESET and TERMINATE though.