Dec 27 2013, 11:52 am
|
|
Nadrew
  |
Looks like SetInfo() is the common link here. The sound Region stuff might also be a culprit, I'd say disable SetInfo() for a bit to see if the crashes stop, if not, disable the sound stuff for a bit; if neither stop it then things are more tricky than you'd expect =P
|
Backtrace for BYOND 503.1222 on Linux: Fri Dec 27 21:09:02 2013 Backtrace for BYOND 503.1222 on Linux: | |
So I think the crashing is indeed happening within /obj/hud/spell_tree/proc/SetInfo. I suspected this was the case back in page 1. Here's what I think is going on inside SetInfo.
| |
I wonder if the maptext still isn't being properly ref-counted. I'll see what I can find there. Is the SetInfo() proc large? If not, perhaps you can post it here since that does seem to be the trigger point.
| |
It's not that big, but it's big enough to warrant me offloading the code to pastebin instead of just pasting it all here. In any case, here's the code.
| |
Just to confirm, all recent crashes usually have SetInfo() called.
Backtrace for BYOND 503.1222 on Linux: Backtrace for BYOND 503.1222 on Linux: Sun Dec 29 21:34:11 2013 Backtrace for BYOND 503.1222 on Linux: Mon Dec 30 23:47:10 2013 Backtrace for BYOND 503.1222 on Linux: Tue Dec 31 12:25:05 2013 Backtrace for BYOND 503.1222 on Linux: Tue Dec 31 18:23:00 2013 Backtrace for BYOND 503.1222 on Linux: | |
The info that this is happening only in certain procs is hugely helpful. It seems SetInfo() is always the culprit here, which I find very interesting. I checked out the line numbers based on the copy of spell_tree.dm that I have, and it seems the DecRefCount error is always happening whenever the Appearance of the image changes, except right after being added to overlays. (This makes sense because when it's added to the overlays, the Appearance's refcount increases and so it isn't being deleted.) The string DecRefCount error is happening when the old Appearance is being deleted.
Appearance doesn't hold any of our Value types, so if a string is having the DecRefCount error, that limits the problem to just seven vars. I can't tell which var is impacting this, so I would suggest that after you create the image in spell_tree.dm, you spit out this output: world.log << "Image created, name=\ref[I:name], desc=\ref[I:desc], suffix=\ref[I:suffix], screen_loc=\ref[I:screen_loc], icon_state=\ref[I:icon_state], text=\ref[I:text], maptext=\ref[I:maptext]" The most important thing to note here is that the DecRefCount errors begin the instant you start changing the Appearance, which suggests that the raw /image type has a string in its default Appearance that has gone bad. To me it suggests that string has been prematurely deleted somewhere else. However a new /image really should have no strings in use at all, unless you've overridden image/New() somewhere or defined some defaults for the /image type. I have not found such a thing in your code; are you using any libraries that might do that? The summon.dm error appears to be interesting as well, although in the code as I have it now, line 34 of summon.dm is ..() in mob/Del(). Can you confirm this? If this is the case, it means the string being deleted is in use in the mob somewhere, or maybe an image using it has been assigned to the mob (perhaps as part of its overlays, or an overlay on a HUD item). | |
Lummox JR wrote:
However a new /image really should have no strings in use at all, unless you've overridden image/New() somewhere or defined some defaults for the /image type. I have not found such a thing in your code; are you using any libraries that might do that? No, we aren't. Should we be using image/New(), instead of just constructing an image with image()? I notice all other methods of maptext we're using seems to be with objects or images constructed with image(). The use of manual construction via new() in spell_tree.dm is an oddity specific to that proc, which happens to the culprit of our crashing. To see if it changes anything, I'm going to just use image(). Can you confirm this? If this is the case, it means the string being deleted is in use in the mob somewhere, or maybe an image using it has been assigned to the mob (perhaps as part of its overlays, or an overlay on a HUD item). I can confirm line 34 is ..(). These are the only strings that are used in Appearances. My hope is that this will print out the refs of the strings involved so that we can match them up to the DecRefCount errors. The string refs should look like [0x600xxxx], and I'll know there's a match when xxxx is the hexadecimal equivalent of the ref you see in the error message. I will apply the changes I mentioned, as well as include the log entry after the new method of image construction. It will take a bit to push these changes to the game and have a crash occur, so a response will be delayed (though not as much as this one). | |
An update on this:
It appears the crashing has stopped after I replaced the assignment of an image variable from new() to image(). This tells me that something is wrong with image/new(). I've replaced this: var/image/I = new() With this:
var/image/I = image(null, layer = FRAME_LAYER + 0.1)
And so far we have not had a single crash. | |
Still haven't crashed since Doohl's changes, and we were going down at least twice daily beforehand--so we can assume with confidence that it's fixed as far as Eternia is concerned.
Good luck with patching the broken code up. Let us know if you need anything further. | |
Lummox JR resolved issue with message:
Calling image/new() with no arguments caused junk data to be used, sometimes resulting in a crash. | |
Looks like we're not in the clear just yet.
Backtrace for BYOND 503.1222 on Linux: | |
Just to be safe, try it with the newest 503 (1224) since that has the fixes Lummox JR mentioned.
| |
I took a look into your latest backtrace. It is not crashing in the same place as the image() bug, but is also clearly not the same bug. It deserves a new report.
What I'm seeing is that the crash is in our PNG read routine, which suggests you either have a malformed icon or ran out of memory. Specifically, the crash happened when trying to assign the red byte to a memory location we allocate during the icon load process, but not while reading that red byte from the memory allocated by the PNG library. This to me suggests it's probably a case of being out of memory rather than the icon being malformed, as a malformed icon I would expect to fail elsewhere. I'm changing those routines (there were some horrible inefficiencies in there) as well as adding a sanity check for the memory allocation; I suspect the latter will solve the problem. Please note the icon itself could still be the "source" of the problem. It may be that you had an icon so large that the attempt to allocate memory to load it failed, but the PNG read structs were created just fine (as those are created first). | |
Just posting here to confirm that Eternia has not crashed in a good two weeks or so since updating to the latest stable build.
| |
Yeah, it was definitely that image/new() call, so good catch in isolating that down. But we definitely want to get a stable 504 Eternia to see how that multithreading works out.
| |