-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix crash on iOS Apple Maps when loading really large polylines #4468
Fix crash on iOS Apple Maps when loading really large polylines #4468
Conversation
This is a very good catch. Good job, @ascherkus! I hope this PR gets merged soon. |
This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Can someone with write access take a look at this PR? |
6eb67d5
to
67b476f
Compare
Rebased my PR on top of latest branch -- would love to get this into the next release if possible. Let me know what's missing or what else I should look at to get this merged. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice optimization.
@Simon-TechForm I can't really vote for approval, as I am no (objective) c expert, but to upvote.
Looks fine to someone who worked a little bit with C and objective C.
@ascherkus any chance we can get that PR rolling again? |
Switch to using heap allocation to prevent corrupting the stack.
67b476f
to
807106a
Compare
Not sure what else to do here -- it's rebased, reviewed, but I can't merge as it's missing one approving review. |
Hello @ascherkus. I sincerely apologize for the late reponse to this pr. The very limited time I've had to spend on this project was put towards migrating to the new react-native arch. I'll test out the proposed changes immediately. |
## [1.5.1-beta.2](v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](#4468)) ([e48e1af](e48e1af))
🎉 This PR is included in version 1.5.1-beta.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [1.6.0](v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](#4468)) ([e48e1af](e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](#4644)) ([8825312](8825312)), closes [#4536](#4536) * **types:** missing PolygonPressEvent type export ([#4410](#4410)) ([d3557a3](d3557a3)) ### Features * enable npm provenance ([#4686](#4686)) ([3498c3f](3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](#4679)) ([661cddb](661cddb))
🎉 This PR is included in version 1.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* chore(deps): bump dev deps * chore(android): rename android package * chore(android): remove redundant class prefix * chore(android): cleanup MapsPackage * chore(android): bump compileSdkVersion and targetSdkVersion to 31 * feat(android): pin androidx.work version Starting this november, all android apps (including updates) will require targetSdkVersion 31 and as a result compileSdkVersion 31, therefore not marking as a breaking change * ci(pullRequest): stop linting pr titles * chore(release): 1.4.0-beta.1 [skip ci] # [1.4.0-beta.1](react-native-maps/react-native-maps@v1.3.2...v1.4.0-beta.1) (2022-10-10) ### Features * **android:** pin androidx.work version ([73f21c7](react-native-maps@73f21c7)) * ci(pullRequest): stop linting pr title * chore(deps): bump dev deps * ci(pullRequest): check formatting * style(prettier): fix formatting * chore(husky): check formatting on pre-commit * build(deps): bump @sideway/formula from 3.0.0 to 3.0.1 in /example Bumps [@sideway/formula](https://github.com/sideway/formula) from 3.0.0 to 3.0.1. - [Release notes](https://github.com/sideway/formula/releases) - [Commits](hapijs/formula@v3.0.0...v3.0.1) --- updated-dependencies: - dependency-name: "@sideway/formula" dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> * chore(deps): bump dev-deps * chore(release): 1.5.0-beta.1 [skip ci] # [1.5.0-beta.1](react-native-maps/react-native-maps@v1.4.0...v1.5.0-beta.1) (2023-04-15) ### Features * **android:** pin androidx.work version ([73f21c7](react-native-maps@73f21c7)) * chore(release): 1.5.0 [skip ci] # [1.5.0](react-native-maps/react-native-maps@v1.4.0...v1.5.0) (2023-04-15) ### Features * **android:** pin androidx.work version ([73f21c7](react-native-maps@73f21c7)) * chore(example): bump to react-native 0.70.8 (react-native-maps#4673) * fix(types): missing PolygonPressEvent type export (react-native-maps#4410) * chore(release): 1.5.1-beta.1 [skip ci] ## [1.5.1-beta.1](react-native-maps/react-native-maps@v1.5.0...v1.5.1-beta.1) (2023-04-16) ### Bug Fixes * **types:** missing PolygonPressEvent type export ([react-native-maps#4410](react-native-maps#4410)) ([d3557a3](react-native-maps@d3557a3)) * fix(ios): crash on Apple Maps when loading large polylines (react-native-maps#4468) Switch to using heap allocation to prevent corrupting the stack. * chore(release): 1.5.1-beta.2 [skip ci] ## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([react-native-maps#4468](react-native-maps#4468)) ([e48e1af](react-native-maps@e48e1af)) * chore: stop updating version in package.json Updating the version requires the Podfile.lock to update after each release. * chore(example): use ruby bundler * chore(example): update gemfile.lock * test(detox): add basic setup * chore(example): default to using patched Google-Maps-iOS-Utils * ci: add build workflow for android and ios * ci: rename build jobs * ci: build ios using frameworks * ci(stale): specify permissions * fix(ios): support for use_frameworks! :linkage => :static react-native-google-maps relies on headers from react-native-maps. When using frameworks, headers from react-native-maps are no longer found, as the framework search path for react-native-maps isn't added to the react-native-google-maps framework. Likewise, removing the import prefix from Google-Maps-iOS-Utils resolves the imports both with and without use_frameworks. Note: this only brings support for `use_frameworks!` with `:linkage => :static. * chore: rename example app * chore(example): migrate to recommended api key setup * ci: add placeholder google maps api key * ci: fix xcworkspace name * chore(release): 1.5.1-beta.3 [skip ci] ## [1.5.1-beta.3](react-native-maps/react-native-maps@v1.5.1-beta.2...v1.5.1-beta.3) (2023-04-18) ### Bug Fixes * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps@b0c2d42)) * feat(ios): bump googlemaps to 7.4.0 (react-native-maps#4679) According to the release notes, this version is functionally identical to v.7.3.0 * chore(release): 1.6.0-beta.1 [skip ci] # [1.6.0-beta.1](react-native-maps/react-native-maps@v1.5.1-beta.3...v1.6.0-beta.1) (2023-04-18) ### Features * **ios:** bump googlemaps to 7.4.0 ([react-native-maps#4679](react-native-maps#4679)) ([661cddb](react-native-maps@661cddb)) * fix(marker): remove spamming warning from MapMarker (react-native-maps#4644) Resolves react-native-maps#4536 * chore(release): 1.6.0-beta.2 [skip ci] # [1.6.0-beta.2](react-native-maps/react-native-maps@v1.6.0-beta.1...v1.6.0-beta.2) (2023-04-18) ### Bug Fixes * **marker:** remove spamming warning from MapMarker ([react-native-maps#4644](react-native-maps#4644)) ([8825312](react-native-maps@8825312)), closes [react-native-maps#4536](react-native-maps#4536) * ci: make reproducible exempt (react-native-maps#4681) * feat: enable npm provenance (react-native-maps#4686) * chore(release): 1.6.0-beta.3 [skip ci] # [1.6.0-beta.3](react-native-maps/react-native-maps@v1.6.0-beta.2...v1.6.0-beta.3) (2023-04-20) ### Features * enable npm provenance ([react-native-maps#4686](react-native-maps#4686)) ([3498c3f](react-native-maps@3498c3f)) * ci(push): add id-token write permissions (react-native-maps#4687) Needed for npm provenance * chore(release): 1.6.0-beta.3 [skip ci] # [1.6.0-beta.3](react-native-maps/react-native-maps@v1.6.0-beta.2...v1.6.0-beta.3) (2023-04-20) ### Features * enable npm provenance ([react-native-maps#4686](react-native-maps#4686)) ([3498c3f](react-native-maps@3498c3f)) * chore: update security policy (react-native-maps#4688) * chore(release): 1.6.0 [skip ci] # [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([react-native-maps#4468](react-native-maps#4468)) ([e48e1af](react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([react-native-maps#4644](react-native-maps#4644)) ([8825312](react-native-maps@8825312)), closes [react-native-maps#4536](react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([react-native-maps#4410](react-native-maps#4410)) ([d3557a3](react-native-maps@d3557a3)) ### Features * enable npm provenance ([react-native-maps#4686](react-native-maps#4686)) ([3498c3f](react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([react-native-maps#4679](react-native-maps#4679)) ([661cddb](react-native-maps@661cddb)) * build(android): bump gradle to 7.2 7.2 adds support for java 17 which is now the default version bundled with android studio * docs(readme): onRegionChangeComplete called infinitely (react-native-maps#4574) * fix(ios): followsUserLocation changes zoom level (react-native-maps#4696) Resolves react-native-maps#4585 * chore(release): 1.6.1-beta.1 [skip ci] ## [1.6.1-beta.1](react-native-maps/react-native-maps@v1.6.0...v1.6.1-beta.1) (2023-04-21) ### Bug Fixes * **ios:** followsUserLocation changes zoom level ([react-native-maps#4696](react-native-maps#4696)) ([3b9491e](react-native-maps@3b9491e)), closes [react-native-maps#4585](react-native-maps#4585) * chore(react-native): bump to 0.70.9 (react-native-maps#4697) * feat(android): bump android-maps-utils to 3.4.0 (react-native-maps#4699) * chore(release): 1.7.0-beta.1 [skip ci] # [1.7.0-beta.1](react-native-maps/react-native-maps@v1.6.1-beta.1...v1.7.0-beta.1) (2023-04-21) ### Features * **android:** bump android-maps-utils to 3.4.0 ([react-native-maps#4699](react-native-maps#4699)) ([6b26c23](react-native-maps@6b26c23)) * chore(prettier): ignore ios/DerivedData (react-native-maps#4702) * chore(release): 1.7.0 [skip ci] # [1.7.0](react-native-maps/react-native-maps@v1.6.0...v1.7.0) (2023-04-23) ### Bug Fixes * **ios:** followsUserLocation changes zoom level ([react-native-maps#4696](react-native-maps#4696)) ([3b9491e](react-native-maps@3b9491e)), closes [react-native-maps#4585](react-native-maps#4585) ### Features * **android:** bump android-maps-utils to 3.4.0 ([react-native-maps#4699](react-native-maps#4699)) ([6b26c23](react-native-maps@6b26c23)) * fix(android): crash when removing feature belonging to collection (react-native-maps#4707) Resolves react-native-maps#4706 * chore(release): 1.7.1 [skip ci] ## [1.7.1](react-native-maps/react-native-maps@v1.7.0...v1.7.1) (2023-04-23) ### Bug Fixes * **android:** crash when removing feature belonging to collection ([react-native-maps#4707](react-native-maps#4707)) ([ae6fe90](react-native-maps@ae6fe90)), closes [react-native-maps#4706](react-native-maps#4706) * chore: merge --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Simon-TechForm <[email protected]> Co-authored-by: semantic-release-bot <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Maxim Artyukhov <[email protected]> Co-authored-by: Andrew Scherkus <[email protected]> Co-authored-by: sharvilak11 <[email protected]> Co-authored-by: Bilal Abdeen <[email protected]>
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
## [1.5.1-beta.2](react-native-maps/react-native-maps@v1.5.1-beta.1...v1.5.1-beta.2) (2023-04-16) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af))
# [1.6.0](react-native-maps/react-native-maps@v1.5.0...v1.6.0) (2023-04-20) ### Bug Fixes * **ios:** crash on Apple Maps when loading large polylines ([#4468](react-native-maps/react-native-maps#4468)) ([e48e1af](react-native-maps/react-native-maps@e48e1af)) * **ios:** support for use_frameworks! :linkage => :static ([b0c2d42](react-native-maps/react-native-maps@b0c2d42)) * **marker:** remove spamming warning from MapMarker ([#4644](react-native-maps/react-native-maps#4644)) ([8825312](react-native-maps/react-native-maps@8825312)), closes [#4536](react-native-maps/react-native-maps#4536) * **types:** missing PolygonPressEvent type export ([#4410](react-native-maps/react-native-maps#4410)) ([d3557a3](react-native-maps/react-native-maps@d3557a3)) ### Features * enable npm provenance ([#4686](react-native-maps/react-native-maps#4686)) ([3498c3f](react-native-maps/react-native-maps@3498c3f)) * **ios:** bump googlemaps to 7.4.0 ([#4679](react-native-maps/react-native-maps#4679)) ([661cddb](react-native-maps/react-native-maps@661cddb))
Switch to using heap allocation to prevent corrupting the stack.
Does any other open PR do the same thing?
No.
What issue is this PR fixing?
No issue filed -- discovered through our own crash monitoring software.
How did you test this PR?
With iOS and Apple Maps load a
<Polyline>
with ~60,000 coordinates and test on a real device (does not seem to crash on iOS Simulator) and deploy / launch debugger using Xcode.Set a breakpoint in AIRMapPolyline.m on setCoordinates().
As soon as the CLLocationCoordinate2D coords array is allocated continuing execution raises a EXC_BAD_ACCESS since the stack has been corrupted.
Switching to heap allocating the temporary coords array allows the polyline to render.
Does not affect Android / Google Maps as that code uses GMSMutablePath and calls addCoordinate in a loop (which presumably is heap allocated under the hood).