[Fixed] Crash with Array::Insert and static event handler

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Crash with Array::Insert and static event handler

Postby Player701 » Sun Oct 04, 2020 4:56 am

The following ZScript will cause GZDoom to crash upon loading a map, provided that the event handler has been registered in MAPINFO:

Code: Select allExpand view
class TestHandler : StaticEventHandler
{
    Array<MyClass> ar;
   
    override void OnRegister()
    {
        ar.Insert(1, new('MyClass'));
    }
}

class MyClass
{   
}

I have attached a WAD to reproduce the crash. Tested in GZDoom 4.4.2 and g4.5pre-230-g89cc69710
You do not have the required permissions to view the files attached to this post.
User avatar
Player701
 
Joined: 13 May 2009
Location: Russia
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: Crash with Array::Insert and static event handler

Postby Major Cooke » Sun Oct 04, 2020 2:26 pm

Does it crash when you do the 'new' first as a variable and then insert it?

Also, just a note, Graf has said OnRegister should be avoided for everything except setting the order.
User avatar
Major Cooke
QZDoom Maintenance Team
 
Joined: 28 Jan 2007

Re: Crash with Array::Insert and static event handler

Postby Graf Zahl » Sun Oct 04, 2020 2:41 pm

Can't check right now, but inserting at index 1 in an empty array surely is undefined behavior at the very best and should probably abort.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Crash with Array::Insert and static event handler

Postby Player701 » Sun Oct 04, 2020 2:45 pm

IMO, it should initialize the elements at all previous indices (0 in this case) with default values (null, 0, false etc. depending on the type). However, an abort would at least be better than a crash. Note that I did this with an array of numeric values previously, and everything worked just fine - of course I never touched the values at indices where I didn't insert anything. It's only logical to expect an object array to behave in the same way.

Major Cooke wrote:Does it crash when you do the 'new' first as a variable and then insert it?

Also, just a note, Graf has said OnRegister should be avoided for everything except setting the order.


It does, because the problem is with the insertion, and not with the generated code. The crash happens during a GC pass.

Regarding OnRegister et al., my experience is that everything will work normally as long as you know what APIs are safe to call at each entry point in your script code. Creating objects and populating an array doesn't involve any external APIs at all, so I don't see any reason why it should suddenly break.
User avatar
Player701
 
Joined: 13 May 2009
Location: Russia
Discord: Player701#8214
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: Crash with Array::Insert and static event handler

Postby Graf Zahl » Sat Oct 17, 2020 4:09 am

fixed
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany


Return to Closed Bugs

Who is online

Users browsing this forum: No registered users and 0 guests