GC randomly collects objects still in use

[Accensus]

This has been tested on both 4.8.2 and GZDoom g4.9pre-214-gba83879e9 by both me and several people. Disclaimer: I am not 100% sure it's a GC issue.

I don't really have a simple test because it's a really specific problem that requires me to post the full mod (earliest I can do is tomorrow), if that'd even be remotely helpful for this. No steps to reproduce because there is no way to reproduce this on the user end in the first place, and that's the painful part. I've been chasing this issue for over four weeks now and it happens completely randomly during gameplay. Any attempts to forcefully reproduce it have been futile. Some people have had it within 30 minutes of starting the game, meanwhile I'm five hours and counting into Extermination Day with no null ref exception in sight.

The gist of it is that mod inventory items that are about to be picked up have an array with data (objects of type ItemData) which is transferred to the array of the item already in your inventory through HandlePickup. Problem: Array.Append copies the references (I cannot use Array.Move). The object that just got picked up gets destroyed immediately afterwards. The data objects that got appended remain in the new array until a random amount of time later when the GC presumably decides to nuke them for no reason, usually either immediately on pickup or several (or many) seconds afterwards. I know for a fact this isn't my code doing that because nowhere in it do I even call Destroy() on ItemData objects, especially in such a random manner, nor do I set core data array elements to null without shrinking the array at the same time, which leaves only GZDoom. Not to mention it doesn't always happen even on the exact same objects in the exact same scenario after, say, loading a save game. There is absolutely no way to predict when it will happen.

[Graf Zahl]

You already nailed the cause - Array<Object>.Append does not perform write barriers on the copied objects. That means they can be collected later at random, depending on the GC's state at the time of the copy. Move has the same issue, btw.

I pushed a fix but you'll have to confirm yourself if it works.
You can manually trigger a collection with "gc now" in the console, that may help you reproduce the problem.

[Accensus]

This is incredible. Thank you! I will test it as soon as a dev build is out. Although looking at the date of the last build, this probably won't be for a day cause I missed the autobuild by an hour, lmao. Ah well. Will report back in a couple of days if nobody on the closed beta team encounters it. Or earlier if we do.

As for "gc now", I tried that several times long before I even wrote the OP, but I couldn't reproduce it that way, unfortunately. Maybe I was misusing it. Gonna give it a couple more tries just to be sure.
Nevermind, I was indeed misusing it. Got it to abort properly. I guess I'll post the results as soon as I can.

[phantombeta]

There seems to be an issue missed here- A lot of the Array<Object> functions are calling the unspecialized template versions of the direct-native functions, which don't have the write barriers, so these will remain broken with the JIT. Strangely enough, Push and Insert had already been done correctly, with specialized functions that call WriteBarrier, but not the rest. (the VM versions of Push and Insert just call the direct-native functions, so they already had the write barriers)
I'll make a PR to fix this.

[Accensus]

Would be greatly appreciated, phantom. I got ahold of a GitHub CI artifact and can still reproduce the issue (given that I'm using the JIT), albeit a little inconsistently.

[phantombeta]

I can't seem to reproduce this with a simple test file, even before Graf's commit. I'll need a file to test this with to make sure the fix works.

[Accensus]

Sent you a PM.

[Gutawer]

phantom - do you have your additional fixes pushed anywhere online? I'm reasonably certain that ZForms gives me a reliable way to test what I believe is this issue, since it causes a bug where rarely elements will call Drawer on a child element, and then the child element will have a NULL master as if the master was destroyed but the array containing it didn't exactly "get the memo" so to speak (either that or a segfault lol).

[phantombeta]

Here's the PR. Seems to fix the issue with the file that was sent to me. I couldn't figure out how to reproduce this with a simple test file, despite many attempts.

One thing I noticed while looking into this bug myself (before Graf pushed his fix) was that TArray::Append uses new(&Array[x]) T(item);, while everything else in TArray uses ::new(&Array[x]) T(item); instead. Does this affect anything? Google says ::new is used to bypass any operator news, so I was wondering if that could affect anything.

[Gutawer]

This doesn't seem to fix my issues with this, though I will admit that it could be a different bug in nature. Here's a runnable sample that can be used to trigger it fairly reliably on my machine: https://www.dropbox.com/s/4rst3qi61gee1 ... g.pk3?dl=0.

To trigger it, open the menu via openmenu TestGenericMenu, then spam click Add New a ton, so much that the elements start to look a bit dumb as the box drawing doesn't really have enough space to work anymore. Then exit the menu and reopen it again, and then do the same thing - keep trying until a crash happens, though normally for me it only takes one reopen. Then you'll get either a VM abort with an abort exception I put into ZForms to detect this bug where it asserts master != NULL (which should always be true if the element's Drawer was called as it's only ever set to NULL in ElementContainer's OnDestroy), or a hard crash - I can only assume both of these are the same issue manifesting in different ways.

EDIT: Been talking to phantom in DMs, fairly sure this is an entirely different issue - probably the same as viewtopic.php?t=74970.

[Accensus]

Messed with the PR build for like ten minutes and it does indeed seem to fix the issues I was having so there's that at least. Many thanks, phantom. I'll give this a lengthy spin and make a post in case I get jumpscared by the console again.

[Talon1024]

Could this issue be what is causing this issue with Raze? According to the ASan report, it does look like an object being collected by the GC which isn't supposed to be.

[Accensus]

This can be closed, the PR has been merged and I can confirm it works fine. Thanks again to everyone involved.