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.

Sign up for GitHub

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 NullPointerException when native class is not present #198

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix NullPointerException when native class is not present #198

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f003a0d
Fix NullPointerException when native class is not present
aceman1209 Jan 4, 2025
2d3b038
Reformat statement
aceman1209 Jan 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Il2CppInterop.Runtime/Runtime/Il2CppObjectPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public static T Get<T>(IntPtr ptr)
unsafe
{
var nativeClassStruct = UnityVersionHandler.Wrap((Il2CppClass*)Il2CppClassPointerStore<T>.NativeClassPtr);
if (!nativeClassStruct.HasFinalize)
aceman1209 marked this conversation as resolved.
Show resolved Hide resolved
if (!nativeClassStruct?.HasFinalize is true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the ! shouldn't be there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

!null is not true. if we get !false it is true like expected. Inversion operates only on bool values.

In this case:

  1. nativeClassStruct checked on null. If it is null - returns Nullable else gets property.
  2. Nullable cannot be inversed by operator !. If property is present - it will be inversed
  3. Comparison with true

But if you mean that the condition itself is false to begin with, then I'm not sure what that field points to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to indicate the presence of a destructor. And if there is no destructor, then we add a reference to the object in GC to finalize it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I initially meant is false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if native class has finalize, we add it to GC, right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

if (!nativeClassStruct.HasFinalize) should be changed to one of the following, and I'm not sure which:

  • if (nativeClassStruct?.HasFinalize is false)
  • if (nativeClassStruct?.HasFinalize is false or null)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second one looks more correct. If I understood the logic correctly - if native class does not have destructor, we add a reference in GC. When native class pointer is null we also need to keep track of the created object. The problem may arise if a class without a pointer has a destructor

Copy link
Collaborator

Choose a reason for hiding this comment

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

@js6pak Which do you think it should be, or how would you suggest we address the problem?

{
Il2CppSystem.GC.ReRegisterForFinalize(newObj as Object ?? new Object(ptr));
}
Expand Down
Loading