Deprecate std.signals?
Ahmet Sait
nightmarex1337 at hotmail.com
Sat May 8 09:37:07 UTC 2021
On Tuesday, 30 March 2021 at 11:23:21 UTC, Berni44 wrote:
> I've got the feeling, that std.signals is rarely used and not
> up to D's standards... Therefore I suggest to move it to undead
> and deprecate it.
>
> But I might err on this. What do you think?
So I did some research on this looking at phobos and a bunch of
other signal/event implementations and concluded that std.signals
isn't even that bad (in fact it's better than some of the stuff
found in the wild). It has issues but they are fixable.
It's nonsensical to me how people want to remove code before
talking about why is it bad and how can it be done better. So
let's do that first.
Issues with std.signals as far as I see:
- Signal is a mixin template
This one wasn't supposed to be a problem but semantics of mixin
templates result in this issue: [Issue 5028 - Problem with named
mixins in base class and derived
class](https://issues.dlang.org/show_bug.cgi?id=5028)
I would be in favor of fixing the semantics for the better
instead of blaming this particular design. Mixin templates even
allow for some neat tricks like making `emit()` function
protected and therefore making it only callable in the class the
signal is declared in (similar to event keyword in C#).
- Signal uses weak reference semantics
This is not actually an issue but a questionable choice
nonetheless. Currently, Signal puts delegates into a malloc-ed
array which is not scanned by the GC and it subscribes to the
object's dispose event using `rt_attachDisposeEvent()`, so when
some object is collected (or manually destroyed), the Signal gets
notified and it removes those delegates which reference that
destroyed object. I don't know what behavior is the better
default but events are strong references where I'm coming from
(C#). Making it optional could be a better decision.
- Signal does not work with lambda delegates
This one is basically a consequence of the previous point and
looks like the biggest reason people handroll their own;
`rt_attachDisposeEvent()` requires an Object and therefore Signal
depends on the delegate context pointer referring to a class
instance. If there was a proper way to know what kind of context
pointer a delegate has, this wouldn't be an issue. See [Issue
19842 - std.signals Segfault when connecting with a
lambda](https://issues.dlang.org/show_bug.cgi?id=19842).
- Making closures capture the variables into an object instance
would solve this for most cases except struct member functions
still won't work. See [Issue 9601 - Make regular D objects on
closures](https://issues.dlang.org/show_bug.cgi?id=9601).
- Introducing different delegate types to the language would
potentially solve this. The idea is to have different delegates
that are compiled down to the same runtime code but allow for
overloading via type system similar to how char & byte types
work. I suspect it is unlikely for this idea to see the light of
day considering the immense bureaucratic nonsense in the DIP
process. Happy to be proven wrong of course.
- Making delegates even fatter by introducing a simple
`ptrType` field so that we can just check it. Not a fan of this
idea.
- Making `rt_attachDisposeEvent()` work with things other than
Object. I have no idea if this is feasible but theoretically the
GC can provide such an API (callback for when a memory region is
collected).
- Alternatively, screw that runtime dispose event feature and
just stop. Make delegates strongly referenced and tell users to
be careful and not destroy/free stuff if it's referenced by
delegates somewhere.
- Have a better idea? Let's see what you got.
- Signal is not thread safe
I don't think this is even relevant. Maybe fix shared first? :P
- Signal.emit() is not reentrant
It currently does not allow recursive calls to the same signal.
Looks like a limitation imposed in order to fix other bugs.
Now let's look at bugzilla:
- [Issue 4150 - std.signals causes memory corruption and
heisenbugs](https://issues.dlang.org/show_bug.cgi?id=4150)
This bug is no longer valid, not reproducable, and should be
closed.
- [Issue 9606 - `std.signal` implementation is fundamentally
incorrect](https://issues.dlang.org/show_bug.cgi?id=9606)
Looks like a valid bug. Signal assumes
`rt_attachDisposeEvent()` callback is not called inside emit().
Related to weakref business above.
- [Issue 16203 - std.signals connect()
error](https://issues.dlang.org/show_bug.cgi?id=16203)
Duplicate of [Issue 19842 - std.signals Segfault when
connecting with a
lambda](https://issues.dlang.org/show_bug.cgi?id=19842).
- [Issue 17011 - cleanup std.signals
documentation](https://issues.dlang.org/show_bug.cgi?id=17011)
Documentation enchanment.
- [Issue 18903 - std.signals uses
_dtor](https://issues.dlang.org/show_bug.cgi?id=18903)
Not actually a bug.
- [Issue 19842 - std.signals Segfault when connecting with a
lambda](https://issues.dlang.org/show_bug.cgi?id=19842)
The biggest actual problem with std.signals as has been
mentioned above.
There is literally nothing interesting about what other
implementations do either. Everyone just does the same thing
creating yet another struct with delegate[] array over and over
again thinking what they do is magically better. Idk maybe that's
what std.signals should do after all ¯\\\_(ツ)_/¯
Let's see what people think.
More information about the Digitalmars-d
mailing list