Apollo-client: Memory leak in client.query()

[JesperWe]

Activate pollInterval for a query.

Intended outcome:

Client can sit idle and do many polls without memory allocation increasing.

Actual outcome:

Allocated memory slowly climbs

How to reproduce the issue:

I am using react-apollo, with a simple query that fetches some counters using pollInterval:

const gqlCounters = gql`
query badgeCounters($accountId: Int!) {
  accountCounters(argId: $accountId) {
    nodes {
      bookingsPreliminary
      bookingsConfirmed
      bookingsForAccount
      unreadMessages
      eventSeq
}}}`;

let countersOptions = {
    skip: ( { accountId } ) => !accountId,
    options: ( { accountId } ) => {
        return { 
            variables: { accountId }, 
            pollInterval: 5000
        }
    },
    props: ( { ownProps, data: { loading, accountCounters, refetch, error } } ) => {
        let data= accountCounters && accountCounters.nodes[ 0 ];
        return ({
            counters: {
                loading,
                error,
                refetch,
                data
            }
        });
    }
};

I am using Chrome. If left idle for a few hours, the tab running the polling will slowly increase its memory allocation until Chrome finally crashes and becomes unresponsive.

Since I am using react-apollo, and the query is always returning the same result, my componentWillReceiveProps() code that handles the result of the query is never even called, so AFAICS this leak is happening inside Apollo itself.

I have tried to trace the leaked memory using the Google "Three Snapshots" technique, and see a lot of unreleased objects looking similar to this one:

[helfer]

This could be pretty much anywhere in the code because polling exercises most parts of Apollo Client. Do you see the same increase if you just run the same query many times "manually" by calling client.query?

[JesperWe]

OK. Seems to be unrelated to pollInterval. I have extrated a small test case app which does absolutely nothing except::

componentDidMount() {
    setInterval( () => {
        this.props.client.query( {
            query: gqlCounters,
            variables: { accountId: 1 },
            forceFetch: true
        })
    }, 200);
}

This is the memory allocation over a six minute period for the Chrome thread running the app:

[helfer]

@JesperWe Thanks! Can you show me the query you're running? I want to make sure this actually maps to the same place in the cache, otherwise the memory usage is expected to grow anyway.

Is there a way of figuring out which objects are being held in memory and where in the code they are being allocated?

[JesperWe]

The query is in the OP. I just made a small app that runs against my dev server for the app I'm working with, exercising only this query.

This is what the query looks like in the Network tab:

The response is:

{
    "data": {
        "accountCounters": {
            "nodes": [{
                    "bookingsPreliminary": 3,
                    "bookingsConfirmed": 0,
                    "bookingsForAccount": 2,
                    "unreadMessages": 0,
                    "eventSeq": 0,
                    "__typename": "Counters"
                }
            ],
            "__typename": "AccountCountersConnection"
        }
    }
}

[JesperWe]

Soo... Your comment about cache mapping made me think. I had just copied all the setup code from my main app into the test app, including the id generation stuff:

let client = new ApolloClient( {
    networkInterface,
    addTypename: true,
    dataIdFromObject: ( result ) => {
        if( result.id && result.__typename ) {
            return result.__typename + result.id;
        }
        return null;
    }
} );

Removing addTypename and dataIdFromObject stops the memory growth, so there is an explanation for you.

I still think this is a bug maybe? Mapping ids like that is needed for queries that fetch objects with ids, and you should still be able to pollInterval a query that has no id related result?

[helfer]

@JesperWe Oh, that's a really interesting clue! So are you saying that even thought the id and typename don't change the memory usage keeps growing if you use dataIdFromObject? (I'd venture a guess that addTypename itself doesn't cause any memory growth).

[JesperWe]

Yes. Well, as you can see this particular query does not even have an ID. I didn't test the two properties individually, but when they are absent I can run forever without leaks, when addTypename and dataIdFromObject are there memory keeps growing even though the query response has no id (so dataIdFromObject should be returning null.

[helfer]

Oh, that's even more interesting. I'll make sure we look into this next week!

[calebmer]

@JesperWe could you test each property individually? I wouldn’t rule out addTypename as the cause yet given that its implementation involves some deep cloning 😉

What happens when there is no addTypename? What happens when there is no dataIdFromObject? Also, is there a small reproduction you could put together for us with instructions on how to hit the leak?

[JesperWe]

I put the test app here (relevant files after creation with create-react-app):

https://gist.github.com/JesperWe/688ce0d78ad3a0aa675169d2aa558453

The server shouldn't matter (I think you know which one I am using ;-)...) just make it respond like above.

[JesperWe]

Nice! @calebmer it turns out you are correct, @helfer guessed wrong. Using:

let client = new ApolloClient( {
    networkInterface,
    addTypename: true
} );

still causes the leak! (no dataIdFromObject)

[calebmer]

So my initial theory is that there is a small memory leak in client.query given that it is implemented using watchQuery which then creates an ObservableQuery etc. effectively exercising most of the code. Since we deep clone the query every time with addTypename the problem becomes much, much worse to the point of becoming noticeable.

This is something that should go away with the major refactors we have planned. How important is getting a timely fix for this issue to you?

[stubailo]

I wonder why something as simple as addTypename would make something this much worse. Could it be a problem in the query transformer itself?

[JesperWe]

Well my project is scheduled to go live in about 4 weeks. Pretty important to me, not sure how important it is to MDG :-)

[stubailo]

Heh, I sure would hope we can fix this in much less than 4 weeks. But any information you can gather will help get it done faster.

[JesperWe]

Hum. In trying to characterize this further, I realize I must backtrack a bit. Earlier I ran the test app for about 6 minutes while observing memory allocation. The I happened to leave it running a lot longer while grabbing a meal, only to come back and find that memory allocation had stabilized. There seems to be a second wave of GC that kicks in after about 8 minutes, and manages to reclaim what the earlier GCs had not.

[JesperWe]

Oh well. I give up. Since yesterday I am no longer reproducing this. The test case runs fine in all circumstances and my main app (which I haven't modified) no longer crashes. Gah.

Ghost in the machine? Silent Chrome update that fixed something? (56.0.2924.87 was released last week with a loong changelog... Not sure when I got it.)

I dunno, I hate myself for waving red herrings :-( Sorry guys.

[calebmer]

It’s important to hear reports like this, even if they are red herrings 😊. Thanks @JesperWe