DreamDaemon crashing on Debian

ID:1919359
 
Aug 17 2015, 12:33 pm
Resolved
Dynamic icons caused crashes in some cases when the webclient was in use.
BYOND Version:508.1296
Operating System:Debian 8.1 GNU/Linux
Web Browser:Chrome 44.0.2403.155
Applies to:Dream Daemon
Status: Resolved (508.1297)
This issue has been resolved.
Descriptive Problem Summary:

In a very specific spot in our game's character creation menu (hairstyle selection), DreamDaemon crashes only when run on Linux. The output is as follows:

BUG: Crashing due to an illegal operation!

Backtrace for BYOND 508.1296 on Linux:
Generated at Mon Aug 17 10:25:42 2015

DreamDaemon [0x8048000, 0x0], [0x8048000, 0x804bb24]
libbyond.so [0xf7237000, 0x0], 0x1e2e09
linux-gate.so.1 [0xf775f000, 0xf775fd40], [0xf775f000, 0xf775fd40]
libbyond.so [0xf7237000, 0x0], 0x1e2e09
libbyond.so [0xf7237000, 0x0], 0x1e2f1e
libbyond.so [0xf7237000, 0x0], 0x29bee0
libbyond.so [0xf7237000, 0x0], 0x2a3587
libbyond.so [0xf7237000, 0x0], 0x2a3755
libbyond.so [0xf7237000, 0x0], 0x2a779d
libbyond.so [0xf7237000, 0x0], 0x2b788a
libbyond.so [0xf7237000, 0x0], 0x1f8e4d
libbyond.so [0xf7237000, 0x0], 0x1fd3c5
libbyond.so 0x315680, 0x31579a
libbyond.so 0x2e38c0, 0x2e3ac2
DreamDaemon [0x8048000, 0x0], [0x8048000, 0x804ae34]
libc.so.6 0x19970, 0x19a63 (__libc_start_main)
DreamDaemon [0x8048000, 0x0], [0x8048000, 0x804a731]

Recent proc calls:
/proc/node_clear_loop
/mob/npc/creature/proc/boss_loop
/mob/npc/creature/proc/boss_loop
/mob/npc/creature/proc/boss_loop
/icon/proc/RscFile
/icon/New
/obj/accessory/New
/mob/player/proc/remove_hair
/mob/player/proc/add_hair
/client/Topic
/proc/node_clear_loop
/mob/npc/creature/proc/boss_loop
/mob/npc/creature/proc/boss_loop
/mob/npc/creature/proc/boss_loop
/proc/node_clear_loop
/mob/npc/creature/proc/boss_loop

To help the BYOND developers debug this, please send the above trace as part
of a very detailed bug report: http://www.byond.com/members/?command=view_tracker&tracker=1


I'll package the latest SW code and page it to Lummox immediately. The issue is consistently-reproducible.
Aug 17 2015, 3:27 pm
This will probably be hard for me to reproduce myself, but tracing the code at least suggests the problem is in some icon code that's deprecated, so I think the sensible course of action is to remove all that.
Aug 17 2015, 3:48 pm
The problem code is probably here:

obj/accessory/New(icon, layer, color = null)
    ..()
    var/icon/I = new(icon)
    src.layer = layer

    if(color && color != "#000000")
        I.Blend(color)

    src.icon = I


Do you suggest removing use of the Blend() proc?
Aug 17 2015, 4:07 pm
Hrm. It's probably the creation of the new icon that's doing it. I'll see if I can deal with that as-is, but most likely I'll be pulling the code responsible for the crash anyway.

Here's roughly what's going on: The old webclient had to load all icon files and create 2-byte IDs for each individual icon in them, because it was responsible for sending all icons to the client. The new DSified webclient doesn't deal with any of that crap; all the server has to do is make sure the icon file isn't a .gif or something. However for legacy reasons it's still doing that dissection of the file, and the place where you saw a crash is basically in code related to that.
Aug 17 2015, 4:52 pm
Maybe I'm missing something here, but isn't that trying to set an objects location to an icon?
Aug 17 2015, 5:08 pm
You can override the arguments that new() can take by overriding New(). They changed the location argument to an icon.
Aug 17 2015, 5:13 pm (Edited on Aug 17 2015, 5:19 pm)
atom.New() is a bit of a special case though.

From the reference:

'By the time New() is called, the object has already been created at the specified location and all of its variables have been initialized. You can perform additional initialization by overriding this procedure.

Since the initial location parameter passed to new() is applied before New() is even called, there is some special handling of the loc variable when using named arguments in a call. Normally, if a procedure is overridden, named arguments in a call are matched against those in the the overridden definition. In this case, however, the loc parameter name is hard-coded. Regardless of what you call the first argument in your definition of New(), the initial location will be taken from the first positional argument, or from the argument named loc if there are no positional arguments. '
Aug 17 2015, 5:26 pm
If the first argument isn't a valid location for the atom, it won't be treated as one.
Aug 17 2015, 5:27 pm
Works for me :)
Aug 18 2015, 2:08 pm
Good news: I found I was able to replicate the crash conditions, and I'm trying to figure out exactly how it's getting into that state. It seemed to happen only when I played with the hair a lot, not right away.
Aug 19 2015, 2:58 pm
Lummox JR resolved issue with message:
Dynamic icons caused crashes in some cases when the webclient was in use.