zoontek / react-native-permissions

An unified permissions API for React Native on iOS, Android and Windows.
MIT License
4.1k stars 836 forks source link

fix: using _handlers instead of self.handlers #788

Closed adamivancza closed 1 year ago

adamivancza commented 1 year ago

Summary

I've noticed the following crash in our app which is happening randomly:

EXC_BAD_ACCESS: Attempted to dereference garbage pointer 0xa1353808b2a0.

0  libobjc.A.dylib +0x2808        _objc_msgSend
1  CoreFoundation +0x9f54         _mdict_removeObjectForKey
2  xxx +0x86f4            -[RNPermissionsModule unlockHandler:] (RNPermissionsModule.mm:351:5)
3  xxx +0x9308            __49-[RNPermissionsModule checkNotifications:reject:]_block_invoke (RNPermissionsModule.mm:427:5)
4  xxx +0x7bc0            __63-[RNPermissionHandlerNotifications checkWithResolver:rejecter:]_block_invoke (RNPermissionHandlerNotifications.m)
5  libdispatch.dylib +0x231c      __dispatch_call_block_and_release
6  libdispatch.dylib +0x3ea8      __dispatch_client_callout
7  libdispatch.dylib +0xb530      __dispatch_lane_serial_drain
8  libdispatch.dylib +0xc0d4      __dispatch_lane_invoke
9  libdispatch.dylib +0x16cd8     __dispatch_workloop_worker_thread
10 libsystem_pthread.dylib +0xdd8 __pthread_wqthread

So I went ahead and checked out the function. I noticed that it uses self.handlers instead of _handlers - I assume that could be a reason for this crash because, in the prior line, we only check if if (_handlers != nil) { and we use _handlers respectively everywhere.

Test Plan

What's required for testing (prerequisites)?

What are the steps to test it (after prerequisites)?

Compatibility

OS Implemented
iOS ✅❌
Android ✅❌

Checklist

zoontek commented 1 year ago

Hi! _handlers is actually just an alias for self.handlers, so I don't think this PR would have an impact. But I can merge it as it's better consistency given the condition just above.