ID:1919359
 
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.
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.
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?
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.
Maybe I'm missing something here, but isn't that trying to set an objects location to an icon?
You can override the arguments that new() can take by overriding New(). They changed the location argument to an icon.
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. '
If the first argument isn't a valid location for the atom, it won't be treated as one.
Works for me :)
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.
Lummox JR resolved issue with message:
Dynamic icons caused crashes in some cases when the webclient was in use.