Warning reference will be nullptred bug

nanashi88 · October 19, 2017, 12:12pm

Hi.

I have described the problem here:
https://answers.unrealengine.com/questions/716269/warning-reference-will-be-nullptred.html

now I think it is a bug, so I am reporting it here, with a way to regenerate it.
the problem is that some minor changes to base class may cause some pointers (of this class or its derived classes ) to be nullptred after build is done.
this happens onley in the current opened level.
other instances in other levels seems to be fine.

this happens when I get a warning like this one:

LogHotReload: Re-instancing MyActor after hot-reload.
LogProperty: Warning: Serialized Class /Engine/Transient.HOTRELOADED_MyActorDerived_2 for a property of Class /Script/test5.MyActor. Reference will be nullptred.
    Property = ObjectProperty /Script/test5.MyActor:ptr
    Item = HOTRELOADED_MyActorDerived_2 /Game/kk.kk:PersistentLevel.MyActorDerived_1

Steps to Reproduce this bug:

Launch the editor and create a new empty C++ project (Basic code).
Add a new C++ class derived from Actor (call it MyActor).

Modify MyActor.h to include the following in the class decleration:

  public:
	UPROPERTY(EditAnywhere, Category = ttr)
	class AMyActor* ptr;
	UPROPERTY(EditAnywhere, Category = ttr)
	bool bValue;

Build the project in Visual Studio and wait for the UE4 editor to hot-reload the updated class.
Back in the UE4 editor, create a new c++ class derived from MyActor (call it MyActorDerived).
in UE4 Editor Create new Level (call it myLevel). open myLevel.
Drag an instance of MyActor class into myLevel (it is automatically called MyActor1).
Drag an instance of MyActorDerived class into myLevel (it is automatically called MyActorDerived1).
Select MyActor1 (from world outliner), go to Ttr category (in the object properities) click on Ptr and from the list choose MyActorDerived1 .
so now MyActor1::ptr pointing to MyActorDerived1
Save all.
Go to visual studio to MyActor.h. rename bValue to bValue2.
(this minor change to MyActor will produce the bug !)
Build the project in Visual Studio and wait for the UE4 editor to hot-reload the updated class.
Back in the UE4 editor, select MyActor1 , you will notice that the pointer “ptr” nullptred and lost its data (of Step 9) .
Open Output Log , you supposed to see the following warning:

LogHotReload: Re-instancing MyActor after hot-reload.
LogProperty: Warning: Serialized Class /Engine/Transient.HOTRELOADED_MyActorDerived_2 for a property of Class /Script/test5.MyActor. Reference will be nullptred.
    Property = ObjectProperty /Script/test5.MyActor:ptr
    Item = HOTRELOADED_MyActorDerived_2 /Game/myLevel.myLevel:PersistentLevel.MyActorDerived_1

this bug may cause big miss when level has many pointers , like our case. waiting for fix. Thank you.

nanashi88 · October 19, 2017, 12:49pm

note: to keep reproducing it, go back to step 9.
and in step 11 every time rename to something else.

Doug_E · November 9, 2017, 8:26pm

Hey nanashi88-

Thank you for submitting a bug report. I have reproduced this issue and logged a report for it here Unreal Engine Issues and Bug Tracker (UE-52220) . You can track the report’s status as the issue is reviewed by our development staff. Please be aware that this issue may not be prioritized or fixed soon.

Cheer

Doug E

nanashi88 · November 13, 2017, 4:26am

Hi Doug E,
Thanks for response.

a small note about the bug report:

Result: MyActorPtr for MyActor1 is
reset to none and RandomNum2 is reset
to zero after the hot reload.

Expected: The variable values remains
after the hot reload.

I think it is ok that RandomNum2 is reseted to zero , because you renamed it, then it is considered a new variable.
then this expected (for me). but what is not expected that MyActorPtr is reseted while no body touch it !

thanks

perky · February 1, 2018, 6:16pm

I have reproduced this issue, the same thing happens with just UPROPERTY floats. Adding, removing or changing any UPROPERTY will reset all other UPROPERTY floats back to their default value. Interestingly, If I load a different map and then load the other map back, the values are back to the values I set before the hot-reload.
(UE 4.18.3)

nocivus · April 2, 2018, 7:07pm

Still waiting for this…

Huogo · April 21, 2018, 3:31am

Something I discovered playing around with the problems associated with this bug. After triggering the Hot Reload, and the values are set to default. If you swap maps, then swap back to the map you were on, any variables whose names did not change will regain the proper value.

If the variable’s name has changed then it still remains at default as expected, but, if you rename the variable back to what it was, hot reload, and to the map swap, it’s value actually goes back to what it was previously (as long as you haven’t made 2 hot reload edits).

chriscummings · May 11, 2018, 9:31am

Hi

This has been a real pain for us so I did some debugging. I have found that most of the hot reload process seems to proceed as expected (or at least, as far as I can infer what is expected), however when we hit UEngine::CopyPropertiesForUnrelatedObjects, the class of the old object has lost references to any properties defined in the C++ super class.

i.e. lets assume we have a c++ class, “MyGameModeBase”, and a blue print, “BP_MyGameMode”. If we modify “MyGameModeBase” and click the cog to hot reload, the “BP_MyGameMode” class will be correctly detected, submitted for reinstancing, recompiled and eventually for each object of type “BP_MyGameMode”, a new instance will be created and “CopyPropertiesForUnrelatedObjects” will be called. This, I assume is all correct.

However, if we allow the function to dump the properties of both the old and new objects, the old class will have no record of any of the properties of “MyGameModeBase”, where the new class will. I believe as a result of this, the property values are not copied from the old instance to the new instance, and thus get reset to defaults.

It’s also worth noting that any objects that are currently not loaded will not be reinstanced at this point (obviously!). This explains why it works if not hot reloading (or indeed, if you hot reload before loading the object). When the editor is reloaded, and you open the object, unreal seems to pick up on the fact that its type is out of date and triggers a reinstance. As before, this calls CopyPropertiesForUnrelatedObjects, however in this case both the old and new object classes have a correct list of properties.

I hope this accelerates the process of this being fixed. Whilst I will continue to dig, I find it amazing that this isn’t considered extremely high priority. A reproducible bug that causes extensive loss of data as part of the core workflow in a content creation package. As a game developer, we wouldn’t even consider shipping something with this kind of floor, let alone ignore it. And the fact that a coder with only a month of Unreal experience can get as far as the above in a few hours kind of suggests an experienced engine programmer should be able to work out the problem.

-Chris

chriscummings · May 11, 2018, 11:01am

Update, following it through the issue seems to be caused by the fact that the data is dodgy when UClass::Link is called. In my current build it gets called no less than 11 times for changing the 1 class, multiple times for each of SKEL_myblueprint, REINST_SKEL_myblueprint, myblueprint and REINST_myblueprint.

The REINST_ versions seem to be the old versions of the blue print classes. They derived from HOTRELOAD_ versions of my c++ class, which appear to be the old versions of the c++ class. However, these HOTRELOAD_ versions have a NULL pointer for their ‘Children’ linked list. As such, the link function assumes the old c++ class has no properties.

Throughout the process the linking also occurs for the new classes, and in these cases they do have a pointer for their ‘Children’, and thus the link function detects the super class’s properties correctly.

I have also verified that the old HOTRELOAD_ class definitions are still invalid at the point at which the CopyPropertiesForUnrelatedObjects is called.

Thus I assume the issue is that the Children pointer is being unnecessarily cleared.

chriscummings · May 11, 2018, 11:34am

Update again. So it appears the culprit code triggering the issue is this

#if WITH_HOT_RELOAD
			// Mark existing class as no longer constructed and collapse the Children list so that it gets rebuilt upon registration
			if (ClassToHotReload)
			{
				ClassToHotReload->ClassFlags &= ~CLASS_Constructed;
				for (UField* Child = ClassToHotReload->Children; Child; )
				{
					UField* NextChild = Child->Next;
					Child->Next = nullptr;
					Child = NextChild;
				}
				ClassToHotReload->Children = nullptr;
			}
#endif

in UObjectBase.cpp, UObjectCompiledInDefer. I am not sure what ‘rebuilt upon registration’ is referring to, so can’t infer whether this is the correct behavior, and something later is failing, or if there is simply a bug in this bit of code. Perhaps it is supposed to be added to the “DeferredCompiledInRegistration” list?

It appears that commenting out the clearing of the child list (but leaving in the clear of the constructed flag) fixes the problem, at least in my minor test case. Whether it survives across more of the project remains to be seen.

Hopefully the unreal team could now take a look and work out what the proper fix is?

cheers

Chris

chriscummings · May 11, 2018, 11:38am

fyi, here’s my version of the working function in case anybody wants it.

arrpeatwo4 · May 11, 2018, 2:36pm

I haven’t tested your fix, but I’m giving you an upvote for due diligence here. I’d become numb to this issue, by begrudgingly working around it every time, but your post really renewed my feelings for it.

I’m still utterly confounded that this is not a top priority issue for Epic. It’s a data-loss issue, and an absolute impediment in the core workflow of Unreal, there’s no other way to look at it. Yes, there’s a workaround, but it’s extremely onerous, and it corrupts the workflow.

I just really have to question what is going on, when an issue as severe as this one persists for multiple releases, and hotfixes with zero attention, and a coder with little experience in the engine, can roll up his sleeves and track down the issue in a couple of nights. Seriously mind-boggling.

I’m going to check your fix later tonight, and reply back here. Thanks again for taking the time to dig into this.

Steve_Robb · May 15, 2018, 3:34pm

Hi all,

The bug was fixed some time ago, but didn’t make it into the 4.19 release. It will make it into the 4.20 release though.

For a fix in the meantime, your UObjectCompiledInDefer() function should look like this:

void UObjectCompiledInDefer(UClass *(*InRegister)(), UClass *(*InStaticClass)(), const TCHAR* Name, const TCHAR* PackageName, bool bDynamic, const TCHAR* DynamicPathName, void (*InInitSearchableValues)	(TMap<FName, FName>&))
{
	if (!bDynamic)
	{
#if WITH_HOT_RELOAD
		// Either add all classes if not hot-reloading, or those which have changed
		TMap<FName, FFieldCompiledInInfo*>& DeferMap = GetDeferRegisterClassMap();
		if (!GIsHotReload || DeferMap.FindChecked(Name)->bHasChanged)
#endif
		{
			FString NoPrefix(RemoveClassPrefix(Name));
			NotifyRegistrationEvent(PackageName, *NoPrefix, ENotifyRegistrationType::NRT_Class, ENotifyRegistrationPhase::NRP_Added, (UObject *(*)())(InRegister), false);
			NotifyRegistrationEvent(PackageName, *(FString(DEFAULT_OBJECT_PREFIX) + NoPrefix), ENotifyRegistrationType::NRT_ClassCDO, ENotifyRegistrationPhase::NRP_Added, (UObject *(*)())(InRegister), false);

			TArray<UClass *(*)()>& DeferredCompiledInRegistration = GetDeferredCompiledInRegistration();
			checkSlow(!DeferredCompiledInRegistration.Contains(InRegister));
			DeferredCompiledInRegistration.Add(InRegister);
		}
	}
	else
	{
		FDynamicClassStaticData ClassFunctions;
		ClassFunctions.ZConstructFn = InRegister;
		ClassFunctions.StaticClassFn = InStaticClass;
		if (InInitSearchableValues)
		{
			InInitSearchableValues(ClassFunctions.SelectedSearchableValues);
		}
		GetDynamicClassMap().Add(FName(DynamicPathName), ClassFunctions);

		FString OriginalPackageName = DynamicPathName;
		check(OriginalPackageName.EndsWith(Name));
		OriginalPackageName.RemoveFromEnd(FString(Name));
		check(OriginalPackageName.EndsWith(TEXT(".")));
		OriginalPackageName.RemoveFromEnd(FString(TEXT(".")));

		NotifyRegistrationEvent(*OriginalPackageName, Name, ENotifyRegistrationType::NRT_Class, ENotifyRegistrationPhase::NRP_Added, (UObject *(*)())(InRegister), true);
		NotifyRegistrationEvent(*OriginalPackageName, *(FString(DEFAULT_OBJECT_PREFIX) + Name), ENotifyRegistrationType::NRT_ClassCDO, ENotifyRegistrationPhase::NRP_Added, (UObject *(*)())(InRegister), true);
	}
}

Hope this helps,

Steve

arrpeatwo4 · May 15, 2018, 3:49pm

Thank you, Steve. I will try to implement this tonight, and appreciate you taking the time to respond here with some information as well as working fix.

Very much appreciated.

AssemblerJohn · May 16, 2018, 6:50am

Thank you! Finally! It was quite a bit of a wait but it surely is worth it!

nanashi88 · May 27, 2018, 10:22am

Hi all,

I haven’t follow this issue for a while. today I logged-in to my e-mail then noticed that there are updates on this issue,
after taking a look, I have noticed the following strange things:

11/5/2018:

after a long long time finaly someone (Chriscummings) decided dig into this issue.
after little time he discovered the Bug source and posted a solution.
then arrpeatwo4 posted a blame on UE.

15/5/2018:

finaly UE staff posted a solution for this Bug.
the bug source is the same one that discovered by Chriscummings.
but still they claim :
“The bug was fixed some time ago, but didn’t make it into the 4.19 release. It will make it into the 4.20 release though.”

then strange thing happened (I don’t know exactly when but it is true to this date) :
the issue of this bug (UE-52220) disappeared.
why it is disappeard instead of it being tagged “Fixed” as normal ?!
actually it was tagged as “Fixed” and here is a proof:
https://web.archive.org/web/20180515203503/https://issues.unrealengine.com/
as u can see from this archived page at 15/5/2018, our issue “Hot reload causes placed actor to lose variable references”
is latest issue that was tagged with “Fixed”.
then this was done recently, and for sure it is after 8/5/2018, proof:
https://web.archive.org/web/20180508103241/https://issues.unrealengine.com/
then why did it disappeared? is it to hide the “Resolved” date?! which is a field inside every issue record!!

18/5/2018:

I received the following e-mail from epicgames:
“We detected a series of unsuccessful login attempts for your Epic Games account.”
WOW, who was trying to hack my account ??! I hope it is not related to this topic
now I wonder, is this post gonna be removed?
I hope UE staff answer this suspictions instead,
and if I was true, I hope they at least say Thank you for Chriscummings instead of this childrish behavior.

AssemblerJohn · May 28, 2018, 5:05am

Yea, weird that they deleted it.

However the fix from Chris was not the same with the fix they (UE guys) did, I’ve personally checked it and it is different.

But yes, the issue should not be deleted, I’ve just saw it is deleted now, it was ‘fixed’ some time ago.

nanashi88 · May 28, 2018, 6:22am

Hi AssemblerJohn, I didn’t say they have the same fix, what I have concentrated on, is the “bug source”, because apparently Chris is the one who found the source of the problem, and even posted a solution before them. so why did they claim that they fixed it even before 4.19 … ?! is it to save face ?!

AssemblerJohn · May 28, 2018, 6:24am

Beats me… However yes, it is a big issue, and it’s core part of the engine, there’s no excuse to have this bug for 8 months with no fix.

arrpeatwo4 · May 28, 2018, 1:01pm

nanashi88, here’s my take on the situation. My assumption is that this bug was introduced as the result of a merge error in the engine source control. If you head over to the official Github for Unreal Engine, you can view the source code and individual commits for the entire engine, as well as for each version of the engine released.

The offending code that ChrisCummings tracked down first appears in version 4.17. I believe the issue was then filed and we got an extremely half-hearted response from Epic staffer Doug E. I’m willing to wager that an Epic engine programmer was able to track the issue down and resolve it on a bugfix branch fairly quickly, because these types of merge errors where old code, gets intermingled with new code in a way that causes it to still compile, but introduce a major bug are not terribly common, but do sometimes occur.

Now, if you look at Doug E.'s profile, you’ll notice it’s no longer active, and it is my assumption that he left the company and this issue was never transferred to another community manager, which led to it losing visibility and persisting for 3 full releases. I think after Chris’s post we got lucky, and it bubbled up to Epic staff who were able to see what happened, and realized the bugfix branch was never merged into the release branch. This is probably what was meant in the response about having “fixed this issue a long time ago.”

It was a major workflow error, but it does in fact happen with these massive software projects, so I’m kind of just glad it was resolved. As far as that account login thing happening, that’s pretty scary, but there is this (Asistencia de Fortnite | Ayuda y servicio de atención al cliente para Fortnite). I’ve read a few posts about some accounts getting hacked, so keep your passwords secure.

Hope this helps,

-Ron