Skip to content
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

PushNotificationIOS loses reference to listeners when adding multiple #21618

Closed
3 tasks done
carbonjesse opened this issue Oct 10, 2018 · 5 comments
Closed
3 tasks done
Labels
Platform: iOS iOS applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.

Comments

@carbonjesse
Copy link

carbonjesse commented Oct 10, 2018

Environment

React Native Environment Info:
System:
OS: macOS High Sierra 10.13.6
CPU: x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
Memory: 46.90 MB / 16.00 GB
Shell: 5.3 - /bin/zsh
Binaries:
Node: 10.8.0 - ~/.nvm/versions/node/v10.8.0/bin/node
Yarn: 1.9.4 - ~/.yarn/bin/yarn
npm: 6.2.0 - ~/.nvm/versions/node/v10.8.0/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
Android SDK:
Build Tools: 23.0.1, 24.0.2, 25.0.0, 25.0.2, 25.0.3, 26.0.0, 26.0.2
API Levels: 23, 24, 25, 26, 27
IDEs:
Android Studio: 3.1 AI-173.4907809
Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
npmPackages:
@storybook/react: ^3.4.0 => 3.4.10
@storybook/react-native: ^3.4.0 => 3.4.10
@types/react: ^16.0.0 => 16.4.13
react: 16.6.0-alpha.8af6728 => 16.6.0-alpha.8af6728
react-native: 0.57.2 => 0.57.2
npmGlobalPackages:
react-native-cli: 2.0.1

Description

Calling

PushNotificationIOS.addEventListener("notification", this.myCallback);
PushNotificationIOS.addEventListener("notification", this.myCallback2);

PushNotificationIOS.removeEventListener("notification", this.myCallback);
PushNotificationIOS.removeEventListener("notification", this.myCallback2);

causes two listeners to be created via PushNotificationEmitter.addListeners.
However since they are tracked in PushNotificationIOS.js in the _notifHandlers map, the second call overwrites the first.

So the first removeEventListener fails to remove the listener. It is an orphaned reference and can cause memory leaks.

https://github.com/facebook/react-native/blob/master/Libraries/PushNotificationIOS/PushNotificationIOS.js#L247

I believe I can fix this if it is indeed a bug/improvement that should be fixed.

Reproducible Demo

Calling addEventListener twice, followed by removeEventListener twice should show this error.

@react-native-bot
Copy link
Collaborator

It looks like you are using an older version of React Native. Please update to the latest release, v0.57 and verify if the issue still exists.

The ":rewind:Old Version" label will be removed automatically once you edit your original post with the results of running react-native info on a project using the latest release.

@carbonjesse
Copy link
Author

On 57.2

@carbonjesse
Copy link
Author

React Native Environment Info:
System:
OS: macOS High Sierra 10.13.6
CPU: x64 Intel(R) Core(TM) i7-6820HQ CPU @ 2.70GHz
Memory: 364.23 MB / 16.00 GB
Shell: 5.3 - /bin/zsh
Binaries:
Node: 10.8.0 - ~/.nvm/versions/node/v10.8.0/bin/node
Yarn: 1.9.4 - ~/.yarn/bin/yarn
npm: 6.2.0 - ~/.nvm/versions/node/v10.8.0/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 11.4, macOS 10.13, tvOS 11.4, watchOS 4.3
Android SDK:
Build Tools: 23.0.1, 24.0.2, 25.0.0, 25.0.2, 25.0.3, 26.0.0, 26.0.2
API Levels: 23, 24, 25, 26, 27
IDEs:
Android Studio: 3.1 AI-173.4907809
Xcode: 9.4.1/9F2000 - /usr/bin/xcodebuild
npmPackages:
@storybook/react: ^3.4.0 => 3.4.10
@storybook/react-native: ^3.4.0 => 3.4.10
@types/react: ^16.0.0 => 16.4.13
react: 16.6.0-alpha.8af6728 => 16.6.0-alpha.8af6728
react-native: 0.57.3 => 0.57.3
npmGlobalPackages:
react-native-cli: 2.0.1

@carbonjesse
Copy link
Author

Having trouble getting rid of the 'Old Version' tag

@react-native-bot react-native-bot added the Ran Commands One of our bots successfully processed a command. label Nov 3, 2018
@react-native-bot
Copy link
Collaborator

I am closing this issue because it does not appear to have been verified on the latest release, and there has been no followup in a while.

If you found this thread after encountering the same issue in the latest release, please feel free to create a new issue with up-to-date information by clicking here.

@facebook facebook locked as resolved and limited conversation to collaborators Nov 3, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Nov 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: iOS iOS applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

2 participants