Bug 15341 – segfault with std.signals slots

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
x86
OS
Linux
Creation time
2015-11-16T02:52:00Z
Last change time
2016-10-01T11:45:47Z
Assigned to
hbaelx
Creator
lt.infiltrator

Comments

Comment #0 by lt.infiltrator — 2015-11-16T02:52:38Z
Modifying the example code from http://dlang.org/phobos/std_signals.html to add a second slot causes a segfault. Tested in 2.066 and 2.068 on X86 Linux, but I doubt that it's platform-dependent. ============== import std.signals; import std.stdio; class Observer { void watch(string, int) { } void watch2(string, int) { } } class Foo { int value(int v) { emit("setting new value", v); return v; } mixin Signal!(string, int); } void main() { auto a = new Foo; auto o = new Observer; a.connect(&o.watch); a.connect(&o.watch2); // not connecting watch2() or disconnecting it manually fixes the issue a.disconnect(&o.watch); // NOT disconnecting watch() fixes the issue destroy(o); // destroying o should automatically disconnect it a.value = 7; // should not call o.watch() or o.watch2() } ==============
Comment #1 by hbaelx — 2016-06-19T16:33:09Z
I can confirm this is still happening as of D 2.071.0, also running on 64bit Linux. I don't exactly know why this is happening, but I believe it has something to do with the fact that the destroy function does not destroy de instance right away. I'll try to reproduce in more cases.
Comment #2 by hbaelx — 2016-06-19T21:21:47Z
The problem was that disconnect calls rt_detachDisposeEvent for that object instance regardless of whether there were other connected slots from that instance, thus unhook would not get called so long as a single slot from that object was disconnected. My proposed solution: Original =========== final void disconnect(slot_t slot) { debug (signal) writefln("Signal.disconnect(slot)"); for (size_t i = 0; i < slots_idx; ) { if (slots[i] == slot) { slots_idx--; slots[i] = slots[slots_idx]; slots[slots_idx] = null; // not strictly necessary Object o = _d_toObject(slot.ptr); rt_detachDisposeEvent(o, &unhook); } else i++; } } =========== Solution ========== final void disconnect(slot_t slot) { size_t disconnectedSlots = 0; size_t instancePreviousSlots = 0; for (size_t i = 0; i < slots_idx; ) { if(slots[i].ptr is slot.ptr) instancePreviousSlots++; if (slots[i] == slot) { slots_idx--; disconnectedSlots++; slots[i] = slots[slots_idx]; slots[slots_idx] = null; // not strictly necessary } else i++; } if(instancePreviousSlots == disconnectedSlots) { Object o = _d_toObject(slot.ptr); rt_detachDisposeEvent(o, &unhook); } } ======== Submitting a PR in Github, will update status as soon as it is accepted.
Comment #3 by github-bugzilla — 2016-09-17T00:32:46Z
Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/978574006c8b3c79cdf08eaecac90e45d215fa9f Fixes issue 15341 - detachDisposeEvent called even with remaining connected slots for that instance https://github.com/dlang/phobos/commit/2ccfbcccab6f121681e4c6727389e469cfe850db Merge pull request #4441 from Dechcaudron/15341 Fixes issue 15341
Comment #4 by github-bugzilla — 2016-10-01T11:45:47Z
Commits pushed to stable at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/978574006c8b3c79cdf08eaecac90e45d215fa9f Fixes issue 15341 - detachDisposeEvent called even https://github.com/dlang/phobos/commit/2ccfbcccab6f121681e4c6727389e469cfe850db Merge pull request #4441 from Dechcaudron/15341