transistorsoft / react-native-background-geolocation

Sophisticated, battery-conscious background-geolocation with motion-detection
http://shop.transistorsoft.com/pages/react-native-background-geolocation
MIT License
2.54k stars 424 forks source link

iOS only: TSLocationManager onUpdateCurrentPosition crashing _objc_sync_enter #1963

Closed calebmackdavenport closed 2 months ago

calebmackdavenport commented 2 months ago

Your Environment

export const getCurrentPosition = async (extras = {}) => { try { await PermissionsStore.useLocationPermission(); await initialize(); let resolve; const pendingPromise = new Promise(r => (resolve = r)); _pendingLocationPings.push({ extras, pendingPromiseResolve: resolve }); _emptyLocationPingQueue(); return pendingPromise; } catch {} };

let _queueIsRunning = false; const _emptyLocationPingQueue = async () => { if (_queueIsRunning) return; _queueIsRunning = true;

try { await _processQueue(); } finally { _queueIsRunning = false; } };

const _processQueue = async () => { const pendingPing = _pendingLocationPings.shift(); if (!pendingPing) return; try { const location = await BackgroundGeolocation.getCurrentPosition({ timeout: 30, maximumAge: 5000, desiredAccuracy: 10, persist: true, extras: pendingPing.extras }); pendingPing.pendingPromiseResolve(location); } catch { pendingPing.pendingPromiseResolve(); } return _processQueue(); };


## Expected Behavior
<!--- Tell us what should happen -->
N/A

## Actual Behavior
<!--- Tell us what happens instead -->
N/A

## Steps to Reproduce
<!--- reproduce this issue; include code to reproduce, if relevant -->
1. I have been unable to reproduce this error myself, I am only able to see that our users are experiencing this issue via Bugsnag.

## Context
<!--- What were you trying to do? -->

Investigating an issue that cropped up today on iOS only. Typically we see a very small handful of these errors, like 5 a day. The existence of the error is a clue that we're likely configured incorrectly somewhere, but the spike in occurrences is more of why I'm posting. We're well into the thousands today.

<img width="1281" alt="image" src="https://github.com/transistorsoft/react-native-background-geolocation/assets/30021449/84518dbf-711b-46a9-800d-5cf4244b09ce">

78.7% of the errors come from users on react-native-background-geolocation 4.14.6
21.3% from users on react-native-background-geolocation 4.12.1

36% of users experience this during app startup (below ~10 seconds)
64% experience the reported error some time later (30+ seconds or more)

My assumption is that a recent iOS update was the catalyst, but I don't particularly understand how.

59% on iOS 17.3.1
18% on iOS 17.2.1
6% on iOS 17.4
3.5% on iOS 16.6.1

## Debug logs
<!-- include iOS / Android logs
- ios XCode logs,
- use #getLog #emailLog methods (@see docs)
- Android: $ adb logcat -s TSLocationManager
-->
<details><summary>Logs</summary>

``` <!-- Syntax highlighting:  DO NOT REMOVE -->
App Hang: The app was terminated while unresponsive

0  libsystem_kernel.dylib +0x1bf4   ___ulock_wait
1  libsystem_platform.dylib +0x32d8 __os_unfair_lock_lock_slow
2  libobjc.A.dylib +0x84b0          _objc_sync_enter
3  app +0xfb95c0           -[TSLocationManager queue:type:]
4  app +0xfb5598           -[TSLocationManager onUpdateCurrentPosition:location:type:]
5  app +0xfb5018           __49-[TSLocationManager createLocationChangedHandler]_block_invoke
6  app +0xfcdaf4           -[LocationManager locationManager:didUpdateLocations:]
7  Dynatrace +0xaa5a4               __45-[DTXAutoInstrument setLocationDelegate_DTX:]_block_invoke
8  CoreLocation +0x2ae2c            0x194f44e2c (0x194f447b4 + 1656)
9  CoreLocation +0x29b74            0x194f43b74 (0x194f43a3c + 312)
10 CoreLocation +0x68e0             0x194f208e0 (0x194f20824 + 188)
11 LocationSupport +0xa918          0x1a69f6918 (0x1a69f6858 + 192)
12 CoreFoundation +0x37134          ___CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__
13 CoreFoundation +0x35928          ___CFRunLoopDoBlocks
14 CoreFoundation +0x33828          ___CFRunLoopRun
15 CoreFoundation +0x333f4          _CFRunLoopRunSpecific
16 GraphicsServices +0x34f4         _GSEventRunModal
17 UIKitCore +0x22c89c              -[UIApplication _run]
18 UIKitCore +0x22bed8              _UIApplicationMain

calebmackdavenport commented 2 months ago

Apologies if this is less than ideal of a submission. I do not like opening issues for problems that I cannot consistently recreate.

Happy to be incorrect, but it seems like an iOS ecosystem problem that I'm not able to discern if there's an available resolution for. I noticed 4.15.0 is available, but wanted to post this issue as I was not certain if the concerns addressed in the changelog matched up with what I'm seeing reports of.

christocracy commented 2 months ago

4.15.0 didn't make any changes to iOS. There haven't been changes to iOS in a relatively long time.

christocracy commented 2 months ago

Was this issue closed on-purpose?

calebmackdavenport commented 2 months ago

Yep. I was not aware of a change behind a feature flag where we were calling getCurrentPosition concurrently 🤦🏼‍♂️

Thank you for checking. Apologies for the false alarm.

christocracy commented 2 months ago

where we were calling getCurrentPosition concurrently

Can you show me a quick example of what the javascript for that looks like? The plugin should be able to handle concurrent calls to .getCurrentPosition.

calebmackdavenport commented 2 months ago

Mmm, we have the queue as noted above to avoid this circumstance.

This would've been an example of the code in question that caused the problem:

const _getLocation = async () => {
  const {
    coords: { latitude, longitude, accuracy, heading, speed },
    timestamp
  } = await BackgroundGeolocation.getCurrentPosition();
  return {
    latitude,
    longitude,
    accuracy,
    observed: timestamp,
    heading,
    speed
  };
};

To remedy, we needed to ensure that we were calling our internal getCurrentPosition which uses a queue:

export const getCurrentPosition = async (extras = {}) => {
  try {
    await PermissionsStore.useLocationPermission();
    await initialize();
    let resolve;
    const pendingPromise = new Promise(r => (resolve = r));
    _pendingLocationPings.push({ extras, pendingPromiseResolve: resolve });
    _emptyLocationPingQueue();
    return pendingPromise;
  } catch {}
};

rather than BackgroundGeolocation.getCurrentPosition() directly.

Having multiple instances of BackgroundGeolocation.getCurrentPosition() in various services firing at app launch (or post-authentication), we run into errors like the one above:

1  libsystem_platform.dylib +0x32d8 __os_unfair_lock_lock_slow
2  libobjc.A.dylib +0x84b0          _objc_sync_enter
3  app +0xfb95c0           -[TSLocationManager queue:type:]
4  app +0xfb5598           -[TSLocationManager onUpdateCurrentPosition:location:type:]

So we use the queue (_processQueue function above), which just works through an array to ensure that we resolve BackgroundGeolocation.getCurrentPosition before calling it again.

I am happy to share additional information or configuration we have, but I am not sure what would be helpful.

christocracy commented 2 months ago

You shouldn't need to manage queues in Javascript. The plug-in already does queuing of requests in Obj-c.

I'd really like a repeatable sample of code that reproduces it so I can see what's up.

andrecrimb commented 1 week ago

I'm also having many of these, but still no success in finding reproducible steps.

App Hang TSLocationManager The app was terminated while unresponsive