ZDoom

In general, a destroyed actor should not continue with its Tick function, but doing so by accident should surely not cause crashes. It is a major issue anyway that the sound code is not GC aware but that's quite non-trivial to fix, it is surely easier to prevent such actors from playing sounds.

[quote="Hojo82"]I should note that GZDoom's own fastprojectile.zs (in wadsrc/static/zscript/actors/shared) calls Destroy() in several places in FastProjectile::Tick(). So while this [i]may[/i] be the source of bugs in GZDoom, it doesn't appear to be explicitly prohibited given the pattern in GZDoom's own source code.[/quote]
OK, so I guess this is not illegal after all. Thanks for investigating this issue.

Thanks for the investigation and the fix.

I've opened pull request #1181 containing the s_doomsound.cpp fix that ensures the actors ObjectFlags doesn't contain OF_EuthanizeMe.

Okay, so after some in depth investigation, the bug is that the Beautiful Doom ZScript RealBullet::Tick() call is calling super.Tick() (which is ZDoom's FastProjectile::Tick()). FastProjectile::Tick() sometimes calls Destroy(), which means by the time control returns to RealBullet::Tick(), the object may be flagged for destruction by the GC. RealBullet::Tick() then calls A_StartSound() if the actor is close enough to the current camera, which means a sound is potentially set to play for an actor that is flagged for destruction. By the time sound is being calculated, the actor has been destroyed, resulting in the sound channel having a now invalid pointer to the destroyed actor. I should note that there's a commented out assert in DoomSoundEngine::CalcPosVel() (specifically in the SOURCE_Actor case of the switch statement) that checks that actor is not nullptr. So it seems that this has potentially been an issue for a while.

In the end, all can be avoided by checking bDestroyed in the RealBullet::Tick() override in Beautiful Doom's script code. However, it also seems prudent to add a check in VerifyActorSound() (in s_doomsound.cpp) to verify that sound isn't being added for an object with flag OF_EuthanizeMe.

To the best of my understanding, what's going on is that a Beautiful Doom (hereafter referred to as BD) RealBullet subclass actor is spawned at some point (RealBullet itself being a subclass of ZDoom's FastProjectile) that plays a whizzing sound if the projectile is closer than 192 units to the current camera during a given tic. However, when the sound is being calculated at a given tic, it appears that the actor has been destroyed already.

When GZDoom crashes, looking at the FSoundChan object in the SoundEngine::CalcPosVel() frame, it references BD's whizzing sound, while the actor pointed at by the channel has been clobbered, presumably because it's been destroyed. It seems to me that the best temporary fix is to put an A_StopSound(CHAN_AUTO) in an OnDestroy() override in BD's RealBullet class. However, that seems to put the onus on the script developer to understand GZDoom internals (which could change anyway), when GZDoom should be smart enough to clean up references to a destroyed actor. In my mind, one of the following two things probably ought to happen in GZDoom:
[list=1]
[*]It should prevent complete destruction of an actor until any sound it is playing has finished playing.
[*]It should immediately stop playing all sounds emanating from an actor upon destruction.[/list]
The first solution seems to be the most difficult and also potentially the most problematic from the standpoint of contravening the script writers intentions. The second one would just require the actor code to stop all sounds immediately in Destroy().

Regarding your statement _mental_:
[quote="_mental_"]Most likely, it’s a mod bug. This crash usually indicates that actor destroys itself during execution of its own Tick() function.[/quote]
I should note that GZDoom's own fastprojectile.zs (in wadsrc/static/zscript/actors/shared) calls Destroy() in several places in FastProjectile::Tick(). So while this [i]may[/i] be the source of bugs in GZDoom, it doesn't appear to be explicitly prohibited given the pattern in GZDoom's own source code.

Thoughts?

[quote="_mental_"]Most likely, it’s a mod bug. This crash usually indicates that actor destroys itself during execution of its own Tick() function.[/quote]
I'm sorry to slightly derail this topic, but could you please tell me why it is not safe to destroy an actor in this way? Because I seem to have quite a few calls to Destroy in my Tick overrides in my own code, and I have never got any crashes from that. Perhaps I have misunderstood you somehow? If not, then what should I do in my Tick code instead of calling Destroy when I have determined that I need to instantly remove the actor?

[quote="_mental_"]Most likely, it’s a mod bug. This crash usually indicates that actor destroys itself during execution of its own Tick() function.[/quote]
Very interesting...so basically something within the ACS code destroying itself in the Tick() override (would destroying self during WorldTick() potentially cause this problem?) could be causing this breakage. That definitely gives some ideas of where to look for culprits. I'll bang away at it and see what I can find. :)

Thanks!

Most likely, it’s a mod bug. This crash usually indicates that actor destroys itself during execution of its own Tick() function.

So I was able to build the source using Visual Studio 2019 from https://github.com/coelckers/gzdoom.git and reproduce the problem at commit b11298af (the current HEAD). The error occurs updating sounds, particularly when calculating position and velocity. However, it appears that the actor's sector pointer is invalid. As I'm unfamiliar with the code, I don't know whether this is due to the sector pointer being bad or the actor pointer being bad. Given what I'm seeing in the actor structure, however, it seems like the actor pointer is invalid.

Here's a link to a screenshot from Visual Studio: https://drive.google.com/file/d/1Fh91S_a9NukNj9BMindYkMHW89KbBRRf/view

I created a dump with the heap and uploaded it to my Google Drive as well. Here's a link to that: https://drive.google.com/file/d/1BedjvyhlwjEXJ4GrKe7_E_cr7xAryg0i/view

If I can be of more assistance, let me know. I'm willing to do a guided debugging tour since I have a working build.

I should also add that I'm using the dev build as 4.4.2 has a brutal memory leak that resulted in nearly all of my 64GB of RAM being utilized which apparently was fixed in June.

[quote="_mental_"]Please share saved game and config file.[/quote]
Attached.

[quote="Rachael"]A) I cannot reproduce the crash with my version of GZDoom. Are you using a dev build or are you self-compiling?[/quote]
I'm using the Windows dev build x64 g4.5pre-136.
[quote="Rachael"]B) It appears as though you are -deh'ing a file and it is not in the package you included. I'm starting to suspect that it might be the cause of the crash.[/quote]
It's included in the file in the Google Drive link. I didn't want to include all of the wads, etc. here in the forum as it's about 45MB download and felt it would be best to foot that storage bill myself.

Please share saved game and config file.

A) I cannot reproduce the crash with my version of GZDoom. Are you using a dev build or are you self-compiling?

B) It appears as though you are -deh'ing a file and it is not in the package you included. I'm starting to suspect that it might be the cause of the crash.

Ultimately, we'd still like it if that's the case because fixing a crash (especially a null pointer access, even if it comes from dehacked) is fairly desirable.