Skip to content

Commit

Permalink
fix: prevent test_load from corrupting object list
Browse files Browse the repository at this point in the history
When using `test_load` on a loaded object the object list ends up
corrupted. The details are obscure, but if doing this reproduces the
error:

1. Load any object in memory. Check that both `find_object` and
   `children()` return the single existing object
2. Run `test_load` on the object. Check that `find_object` returns `0`
3. Destroy and create the object. `children()` will now return an array
   with two duplicates of the same object or, sometimes, a `0` and the
   existing copy of the object

In some instances i've seen different objects returned by `children()`
than the one being passed as path (this usually happened with clones).

This PR tries to work around it. I've also reverted #843 because I
believe this PR fixes the underlying cause for the issue, not just the
symptom.

All credit goes to Zoilder (one of the developers on our mud) that
happened to fix this a long time ago. We're just sending these fixes
upstream since we're starting to update our outdated driver.

Please let me know if there's anything I should fix. I've tried to add a
proper test for this, but I don't know if that's the location you're
expecting for it.
  • Loading branch information
hluaces committed Apr 28, 2024
1 parent 062c294 commit ad8b83e
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/packages/contrib/contrib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3039,6 +3039,7 @@ object_t *testloadob;
static void fix_object_names() {
if (testloadob) {
SETOBNAME(testloadob, saved_extra_name);
ObjectTable::instance().insert(testloadob->obname,testloadob);
}
}

Expand All @@ -3047,6 +3048,7 @@ void f_test_load() {
object_t *new_ob;
if ((testloadob = find_object2(sp->u.string))) {
tmp = testloadob->obname;
ObjectTable::instance().remove(testloadob->obname);
SETOBNAME(testloadob, "");
}
saved_extra_name = tmp;
Expand All @@ -3070,6 +3072,7 @@ void f_test_load() {
push_number(1);
if (testloadob) {
SETOBNAME(testloadob, saved_extra_name);
ObjectTable::instance().insert(testloadob->obname,testloadob);
}
}
#endif
2 changes: 2 additions & 0 deletions testsuite/single/tests/crasher/1063-aux.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
void create() {
}
18 changes: 18 additions & 0 deletions testsuite/single/tests/crasher/1063.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
void do_tests() {
object ob, *all;
string path;

path = base_name() + "-aux.c";
ob = load_object(path);
all = children(path);
ASSERT_EQ(sizeof(all), 1);

test_load(path);
all = children(path);
ASSERT_EQ(sizeof(all), 1);

destruct(ob);
all = children(path);
ASSERT_EQ(sizeof(all), 1);
ASSERT_EQ(member_array(0, all), -1);
}

0 comments on commit ad8b83e

Please sign in to comment.