Fix NullPointerException when native class is not present#198
Fix NullPointerException when native class is not present#198aceman1209 wants to merge 3 commits intoBepInEx:masterfrom
Conversation
| { | ||
| var nativeClassStruct = UnityVersionHandler.Wrap((Il2CppClass*)Il2CppClassPointerStore<T>.NativeClassPtr); | ||
| if (!nativeClassStruct.HasFinalize) | ||
| if (!nativeClassStruct?.HasFinalize is true) |
There was a problem hiding this comment.
I think the ! shouldn't be there.
There was a problem hiding this comment.
!null is not true. if we get !false it is true like expected. Inversion operates only on bool values.
In this case:
- nativeClassStruct checked on null. If it is null - returns Nullable else gets property.
- Nullable cannot be inversed by operator
!. If property is present - it will be inversed - 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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Oh, I initially meant is false
There was a problem hiding this comment.
if native class has finalize, we add it to GC, right?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
There was a problem hiding this comment.
@js6pak Which do you think it should be, or how would you suggest we address the problem?
There was a problem hiding this comment.
The real solution is to inject unstripped types, but for now we can just do nativeClassStruct != null && !nativeClassStruct.HasFinalize
|
@aceman1209 How did you encounter this null reference exception? Can you show us a stack trace? |
|
@ds5678 Sorry for the late reply I tested this on Miside. I think any game that doesn't use asset bundles will work. To get the error, you just need to try to load your asset bundle. Unity will load it successfully (trying to load it again will cause an exception that such a bundle is already loaded), but because of the further exception, we will not get an instance of the class. Stack trace. I used MelonLoader for it. |
|
@aceman1209 when I discussed this with @js6pak , we were concerned about two things:
|
|
@ds5678 If I understand correctly, metadata contains information on which unstripped classes are built. But because the required class is not used in the game code, it is not included in the metadata during compilation, since there are no references to it. Correct? |
If you remove "unstripped" from this sentence, everything you said in that message is correct. |
|
Fixed in #247 |
3 participants
A part of this pull request.
Required more in-depth review. Probably unstripped classes do not have a pointer to the native class in Il2CppClassPointerStore context. With UnityEngine.AssetBundleModule.AssetBundle works fine, but i'm not sure about GC Finalize.