[4.13] Random crash with Canvas, maybe

[Axensus]

There's another random crash but it's on a map without Canvas textures so I'll report that separately once I manage to get it to crash again.

Steps to reproduce this one with 100% certainty:
1. Get https://gitlab.com/accensi/deathstrider
2. +set ds_merchant 1 +map flagship +playerclass Wraith
3. Restart the map until the TV to the left of the player plays. This happens when there's a white-haired girl in front of it. Her spot is randomized so restart until she's there.
4. i_timescale 20 (or not, if you feel particularly nostalgic)
5. Wait until the episode ends (roughly 7 in-game minutes) and No Signal is displayed. The crash happens about a second later.

[Axensus]

Would have been great if I didn't forget the crash report.
https://drive.google.com/file/d/14JbV8q ... drive_link

While I'm doubleposting, tested this on both OpenGL and Vulkan. Crash happens on both. GPU is AMD Radeon RX 6800 XT on Win 10.

[Rachael]

This is proving to be very difficult to trigger, especially with a map that is slow to load on my system.

Would you mind putting this in a smaller example where the TV is guaranteed to play on map load, with possibly a smaller map? Whatever method you are using to switch it to "no signal" - keep that - because I suspect it has something to do with this bug.

[dpJudas]

I tried launching this map over 40 times now. Both with 'map flagship' and ' changemap flagship', never seeing the TV display anything else than "no signal". Never saw any crash.

Please keep in mind this is an open source project done by volunteers. The harder you make it for us to reproduce the issue, the less likely anything will be fixed.

[phantombeta]

You destroy the canvas texture here. My immediate guess would be that this isn't something that should be done, but I don't have enough knowledge of the code for canvas textures to know for sure.

[Axensus]

The inability to reproduce was my fault entirely, sorry. I messed up one of the CVars. Either way, I can't make a smaller example because I don't know what exactly goes wrong but I did make a branch without the randomness (CVar is irrelevant now) so it should make things a lot easier and faster to debug. Plus I remembered you can turn off the TV manually.

https://gitlab.com/accensi/deathstrider ... /crashtest
Fresh crash report just in case: https://files.catbox.moe/8j34ot.zip

However, I found some things while testing this on my laptop where it doesn't always crash after the episode stops. If you walk up to the TV and press use on it to turn it off after starting the level, without having to i_timescale or wait, then use "map map01", it'll immediately crash. Same thing happens when you fire the gun a couple of times, but not always. Pretty sure at least twice it also crashed when I opened the main menu after the menu sound played, though both of those cases require waiting longer. No idea anymore.

Hopefully this helps. As for the loading times, yeah, I really ought to do something about that later today.

@phantombeta:
Strangely enough this worked flawlessly until recently so something's changed. And I couldn't find another way to get rid of a canvas texture back when this was written.

[dpJudas]

Nice spot, phantombeta. That Canvas.Destroy() is absolutely not allowed. What is the best way to block that?

[Graf Zahl]

The only way I see is to throw an exception - preferably in the Destroy function, but if that is not doable, when the non-existent camera texture is accessed.

[Axensus]

Guess I'll have to look into an alternative solution on my end then.

[Axensus]

Fixed on my side. Been a while since I last ran into a blatant [Don't do that]. Turns out setting it to null was sufficient. Guess I used Destroy() because the alternative was to use SendNetworkEvent from UI scope to set the play variable to null. Shouldn't have been THAT lazy.

[Axensus]

Should this be closed? As far as I'm concerned, the issue was resolved on the mod's side and I saw Rachael pushed a Canvas-related null check a few days ago, so if this is all taken care of I'll go ahead and delete the test branch. Stale branches bum me out.

[Rachael]

The null check actually doesn't do anything useful other than move the problem somewhere else where it can be traced better. Ultimately though, what seems to be the desired solution has not yet been implemented - and that is the blocking of canvas.Destroy()

[dpJudas]

I think the following might do the trick, but I'm not entirely sure:

Code: Select all

class FCanvas : public DObject
{
...
  void OnDestroy() override { I_FatalError("Calling Destroy on a canvas object is not allowed."); }
};

[Graf Zahl]

I_Error should be enough here.

[Rachael]

Isn't Canvas a static object? I think I'd rather fatal error out to avoid the risk of dangly pointers.