Skip to content

Add null checks, and improve swig error handling #1101

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

Merged
merged 3 commits into from
Sep 4, 2024
Merged

Conversation

a-maurice
Copy link
Collaborator

@a-maurice a-maurice commented Sep 3, 2024

Description

Provide details of the change, and generalize the change in the PR title above.

Add missing null checks to new Messaging conversion logic, and improve the swig conversion logic to better handle if an exception is thrown, to prevent a double free. This was causing a crash when opening a notification.


Testing

Describe how you've tested these changes.

Running locally.
GHA: https://github.com/firebase/firebase-unity-sdk/actions/runs/10691311630


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

#1091

@@ -316,18 +316,22 @@ void SendPendingEvents() {
// messageReceivedDelegate.
[MonoPInvokeCallback(typeof(MessageReceivedDelegate))]
private static int MessageReceivedDelegateMethod(System.IntPtr message) {
int tookOwnership = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this has to be an int vs. a bool, add a comment for why?

@a-maurice a-maurice merged commit 512e201 into main Sep 4, 2024
4 checks passed
@a-maurice a-maurice deleted the am-messaging_fix branch September 4, 2024 00:27
@firebase firebase locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants