Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[ios, macos] Fix a memory leak when creating an EAGLContext in MGLMaSnapshotter #11193

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

fabian-guerra
Copy link
Contributor

Fixes #11117 remaining leak.

@friedbunny
Copy link
Contributor

Could you please briefly explain what the issue was and how this fixes it?

@fabian-guerra
Copy link
Contributor Author

@friedbunny sure.

After profiling MGLMapSnapshotter I realized we had some memory leaks related to retaining self in the dispatch blocks. This was fixed by temporally retaining a weakSelf instance inside the blocks, releasing the underlaying _snapshotCallback and moving the attribution addition to it's own method. This PR addresses those issues #11133.

But there was still a leak that was keeping ~10MB per snapshot creation. The culprit for this leak was an instance of EAGLContext that was kept around. Here the context was created

EAGLContext* glContext = [[EAGLContext alloc] initWithAPI:kEAGLRenderingAPIOpenGLES2];
and passed the ownership to
[reinterpret_cast<EAGLContext*>(glContext) retain];

Since this file headless_backend_eagl is marked as -fno-objc-arc HeadlessBackend::createContext() was not relinquishing its ownership thus making the leak.

This change makes EAGLBackendImpl the creator and owner of the context and the method HeadlessBackend::createContext() no longer holds a reference to it.

@fabian-guerra
Copy link
Contributor Author

@ivovandongen @kkaefer looks like this change that Konstantin did "fixed" part of the problem in master and release-boba branches 335f04f#diff-71f34354e3b569a52e855d04f2bdeff3

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we didn’t mention #11133 in the v3.7.4 release notes, consider release-noting this fix, assuming it eliminates the remaining leakage.

@friedbunny friedbunny added iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage labels Feb 16, 2018
@friedbunny friedbunny modified the milestones: ios-v3.7.5, ios-v3.7.6 Feb 16, 2018
@fabian-guerra fabian-guerra merged commit 6d2cbb0 into release-agua Feb 27, 2018
@fabian-guerra fabian-guerra deleted the fabian-fix-memory-leak-11117 branch February 27, 2018 17:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS performance Speed, stability, CPU usage, memory usage, or power usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants