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

Fix crash in RealmObject finalizer due to null _accessor. #3046

Merged
merged 2 commits into from
Oct 6, 2022

Conversation

elle-j
Copy link
Contributor

@elle-j elle-j commented Oct 6, 2022

Description

Fixes #3045.

When creating a RealmObject that throws before being initialized, _accessor will be null. When the object was garbage collected, RealmObject's destructor would call _accessor.UnsubscribeFromNotifications() causing a crash due to NullReferenceException.

Solution:
RealmObject's destructor was removed and instead added the call to UnsubscribeFromNotifications() in ManagedAccessor's destructor. (For an UnmanagedAccessor, a call to UnsubscribeFromNotifications() does nothing.)

TODO

  • Changelog entry
  • Tests (if applicable)

@elle-j elle-j requested a review from nirinchev October 6, 2022 09:15
@cla-bot cla-bot bot added the cla: yes label Oct 6, 2022
@elle-j elle-j requested review from papafe and LaPeste October 6, 2022 09:15
@peppy
Copy link

peppy commented Oct 6, 2022

Thanks for the fix, it looks solid.

As an aside, this is also really cool to see – finalizers are not zero overhead and we've seen performance degradation from having thousands-millions of objects with finalizers. This was unavoidable with RealmObject derived models, but looks like it may become a reality with this change (for cases of UnmanagedAccessor, which is beneficial to us as we use the same model in non-managed batch processing).

@elle-j
Copy link
Contributor Author

elle-j commented Oct 6, 2022

Thank you @peppy for providing the necessary details in your ticket so that we could provide a fix quicker!

Copy link
Contributor

@LaPeste LaPeste left a comment

Choose a reason for hiding this comment

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

Interesting subtlety.

@elle-j elle-j merged commit fe36a62 into main Oct 6, 2022
@elle-j elle-j deleted the lj/handle-realmobject-init-throws branch October 6, 2022 11:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Crash in finaliser if RealmObject implementation throws in object initialisation
4 participants