23 Mar, 2009, Skol wrote in the 1st comment:
Votes: 0
I get bugs in the logs occasionally so I took a look.
This bug usually occurs on body parts with timers, it's not terribly frequent but I don't like any bugs…

The error:
[*****] BUG: Extract_obj: obj 12 not found.

Other times it's obj 10 (a corpse) or an arm etc etc.

/*
* Extract an obj from the world.
*/
void extract_obj (OBJ_DATA * obj)
{
OBJ_DATA *obj_content;
OBJ_DATA *obj_next;

if (obj->in_room != NULL)
obj_from_room (obj);
else if (obj->carried_by != NULL)
obj_from_char (obj);
else if (obj->in_obj != NULL)
obj_from_obj (obj);

for (obj_content = obj->contains; obj_content; obj_content = obj_next)
{
obj_next = obj_content->next_content;
extract_obj (obj_content);
}

if (object_list == obj)
{
object_list = obj->next;
}
else
{
OBJ_DATA *prev;

for (prev = object_list; prev != NULL; prev = prev->next)
{
if (prev->next == obj)
{
prev->next = obj->next;
break;
}
}

if (prev == NULL)
{
bug ("Extract_obj: obj %d not found.", obj->pIndexData->vnum);
return;
}
}

–obj->pIndexData->count;
free_obj (obj);
return;
}


I'm wondering if something as simple as…
OBJ_DATA *prev;
OBJ_DATA *prev_next
;

Then switching the for loop from:
for (prev = object_list; prev != NULL; prev = prev->next)


To:
for (prev = object_list; prev != NULL; prev = prev_next)


With:
prev_next = prev->next;


Anyway, it's a bug that pops up in Stock RoM/RaM so it'd be great to fix for all.
Thoughts?
24 Mar, 2009, Sandi wrote in the 2nd comment:
Votes: 0
This one has been making me crazy for the last nine months. It's the last extract_obj() in update.c/obj_update that's doing it.

I don't think it's extract_obj() itself, as that works fine elsewhere. It doesn't seem to be in the specials, though I did find a recursive fire_effect(t_obj) at the end of _fire_effect() that comes right after extract(t_obj). Oops!

Most frustratingly, I can't make it happen. It only seems to happen several hours after certain players are on. I think. :)
24 Mar, 2009, Skol wrote in the 3rd comment:
Votes: 0
Rofl, at least it's not just me.
It's one I've seen for like 10 years heh, but… yeah, I'm thinking it's always the last object in the list. 13.5 hours since I've seen one in the log tail, 10-20 players on all day, gaaah.

It's _always_ an item with a timer on it. So not like object in a container vaping with container (corpses with contents etc).

Maybe we can get some more input.
26 Mar, 2009, Sandi wrote in the 4th comment:
Votes: 0
Skol, are you sure it happens in stock ROM? I'd been thinking it was something I did. In, fact, I was sure I'd fixed it once…. as a result, I've trusted extract_obj() was right. Hmmm…


Adding prev_next "shouldn't" make a difference, as we're not removing anything in that loop. (then again, anything's worth a try at this point)

However, above that, I think we might want: obj_content != NULL

Otherwise, it looks like we'll extract a NULL, and I bet you can't find a ->next_content for one of those. :)


BTW, I changed the bug message to be more accurate: "Extract_obj: prev->next not matched for %d."


To really narrow it down, it's always an object created by death_cry.
26 Mar, 2009, Skol wrote in the 5th comment:
Votes: 0
Hey Sandi,
Yeah, I looked at the stock extract_obj() and it's the same (Mine I have money going back into a global GMP (gross mud production), but the one I posted was from stock).
I do hear you on the prev_next = prev->next; basically it will still hit the if it's NULL spit out error message.
I've found it's _always_ a timered item, but not always death_cry on my own game as there are other items that timer. Still rare, but I've seen other items come up as well, always timered items though.

I went through object_update to see if there was maybe two calls to extract_obj(), maybe a missed return, etc etc. Nothing there.

I'm going to experiment with it a bit and see if I can get it to reproduce regularly.
27 Mar, 2009, ghasatta wrote in the 6th comment:
Votes: 0
Based on Sandi's comment, I have been examining obj_update() in update.c. I think this is probably where the problem is, not in extract_obj(). However, before I go any further, I should mention that I think this is a benign situation. I don't think there are any consequences for the overall stability of a game because of this issue, because it is safe to call extract_obj() on an object more than once.

IMO the obj_update() function needs to be blown up. I think the issue is stemming from the fact that operations are being made on the master object list as it is being iterated through. Consider this code at the bottom of obj_update():

if ( ( obj->item_type == ITEM_CORPSE_PC || obj->wear_loc == WEAR_FLOAT )
&& obj->contains )
{ /* save the contents */
OBJ_DATA *t_obj = NULL;
OBJ_DATA *next_obj = NULL;

for ( t_obj = obj->contains; t_obj != NULL; t_obj = next_obj )
{
next_obj = t_obj->next_content;
obj_from_obj( t_obj );

if ( obj->in_obj ) /* in another object */
obj_to_obj( t_obj, obj->in_obj );

else if ( obj->carried_by ) /* carried */
if ( obj->wear_loc == WEAR_FLOAT )
if ( obj->carried_by->in_room == NULL )
extract_obj( t_obj );

else
obj_to_room( t_obj, obj->carried_by->in_room );
else
obj_to_char( t_obj, obj->carried_by );

else if ( obj->in_room == NULL ) /* destroy it */
extract_obj( t_obj );

else /* to a room */
obj_to_room( t_obj, obj->in_room );
}
}

extract_obj( obj );
}


For an object that is to be removed, the above chunk of code iterates through any contained objects, moves the contained object out of the object that is to be removed, and possibly extracts them as well.

Let's say hypothetically that you have a master object list containing just two objects.
The top object, A_obj, is set to expire.
The second object, B_obj is also set to expire, and happens to be contained in A_obj.
A_obj is not in a room.

obj_update() is called, and encounters A_obj first.
At this point, we are saving A_obj->next as 'obj_next'.
We iterate through the contained objects in A_obj, encountering B_obj.
We remove B_obj from A_obj, and then call extract_obj() for B_obj. B_obj is now invalid. A_obj->next is now NULL. obj_next is still B_obj.
Finished iterating through the items contained in A_obj, we now call extract_obj() for A_obj. A_obj is now invalid. B_obj is now invalid. obj_next is still B_obj.
We reach the end of the loop and continue to the next iteration. Our target object for this iteration is set to obj_next, which happens to be B_obj, which has already been invalidated.
We call extract_obj() for B_obj. extract_obj() complains with the message noted in the OP above, since B_obj was already removed from the master object list in the previous iteration of the loop. Othen than spitting out the error, extract_obj() handles the issue gracefully, and life moves on.

One solution is to have items-to-be-deleted to an intermediate set instead of extracting them while iterating through the master object list. A set has the unique property that each element must be unique, so if a pointer is added to the set twice, there will still only be one element in the set for that value. Then, once finished iterating through the master object list, call extract_obj on each item to be removed.

There's my 2 cents and then some. HTH.
27 Mar, 2009, Sandi wrote in the 7th comment:
Votes: 0
ghasatta said:
Based on Sandi's comment, I have been examining obj_update() in update.c. I think this is probably where the problem is, not in extract_obj(). However, before I go any further, I should mention that I think this is a benign situation. I don't think there are any consequences for the overall stability of a game because of this issue, because it is safe to call extract_obj() on an object more than once.


I tend to agree. I'm thinking it's most likely a left-over debugging message that's no longer needed.


(and, thanks for taking the time to work through it)
0.0/7