Apollo-ios: Lots of small memory leaks

[rnystrom]

I'm using codegen 0.18.8 and Apollo 0.8.0 and noticing a huge amount of leaks using the Debug Memory Graph tool. Each of them is 32 bytes and I have 1313 of them, so ~42kb in leaks.

It looks like this all comes from some static selections:

public static let selections: [GraphQLSelection] = [ /* ... */ ];

Is this a concern? If anything, it's eating up a ton of UI when debugging cycles (lots of "load more"). Any tips for working around this?

[martijnwalraven]

Could you see if this has been fixed in 0.9.0?

[js]

Since the static [GraphQLSelection] are still there, these still appear in 0.9.0 as far as. I can see

[viniciussoares]

Same here :(

[viniciussoares]

I think the problem is related to accessing resultHandlers inside lock closures.

https://github.com/apollographql/apollo-ios/blob/master/Sources/Apollo/Promise.swift#L217
https://github.com/apollographql/apollo-ios/blob/master/Sources/Apollo/Promise.swift#L229

[Nickersoft]

Getting this as well on 0.9.1 in the same areas of the codebase. Any update?

[dox-mark]

I'm also getting these leaks.

[gordoneliel]

I'm getting these leaks as well, any update on this?

[NiltiakSivad]

Seeing these as well on 0.9.4

[mohamedEmaad]

Still have this issue

[francybiga]

I'm also getting leaks in version 0.9.4

[claudiagrilli]

Still there on 0.10.1

[pskow]

It's been more than year and this issue is still there :(
IMO it's a serious problem - after using app for few minutes with each api call memory usage increase enormously

[sam961]

same issue here ... I tried to fixed the issue ... but I didn't get any solution :(

[designatednerd]

I'll try to get a look at this - there's definitely some weirdness due to our promises implementation that I want to try to get a better handle on.

[jstngoulet]

Is there an update on this issue

[designatednerd]

Not directly - there's a lot of stuff where we've been able to make incremental progress, but a halfway reasonable amount of it is tied to some architectural things that are going to take us some time to fix. If there's a specific place you're seeing memory leaks, we'd love to have some help tracking things down. Thank you!

[jstngoulet]

Looks like I am having the same issue as the OP in the latest client version, relating to [GraphQLSelection]. I am getting 3 32-byte leaks and 2 48-byte leaks on a single mutation. I can provide additional info if necessary, but appears to be the same issue as this thread already explains.

[designatednerd]

The good news is that the GraphQLSelection stuff is going away in the Swift Codegen Rewrite, outlined in #939. The bad news is as you can tell from the length of that issue, that's gonna be a minute before it's ready

I can try to take a look at some of the locking stuff as we've had something contributed using NSLock under the hood, but I'll preface by saying that's a huge rabbit hole and I don't know if I'll find anything.

[gsabran]

Is this really a memory leak? As those are static variables this is bounded by the number of classes right? Well maybe each parent type is duplicating the objects its compute from its children?

Would changing

public static let selections: [GraphQLSelection] = [ /* ... */ ]

to

public static var selections: [GraphQLSelection] { [ /* ... */ ] }

on children types solve the issue?

[Tr736]

@gsabran Amazing suggestion. This resolved most if not all of my memory leaks

[gdfairclough]

Any updates here? Wondering if the memory leaks have been fixed on a newer version or not.

[designatednerd]

Things are basically still where they are when I last commented - I am working on the codegen rewrite but it's a bit of a slow process.

@gsabran Do you think it's worth looking at changing olde codegen to change those things to vars?

[gsabran]

That looks like a small change with no impact on the public APIs, so if that's an issue for folks maybe?

[designatednerd]

I think maybe we may need to protect it with a private(set) so people don't go doing crazy things in there, but that seems reasonable.

I'll try get a look at this tomorrow

_puts on typescript armor_

[gsabran]

If you implement it as a computed var (not a lazy var), there is no possibility to set a value.

[designatednerd]

@gsabran - started to look into that and went down a very stupid formatting rabbit hole and eventually realized it was going to be a lot easier just to do private(set). Would love a look at the Typescript PR if you have a second.

[alexjameslittle]

@designatednerd Do you know when this issue might be resolved?

[designatednerd]

ahhhh never got the changes from the typescript PR merged in over here, thanks for the reminder.

[designatednerd]

This has, at long last, shipped with 0.29.0.

Thanks to everyone for your patience on this and particularly to @gsabran for spotting the real underlying issue.