Is there any reason why std.signals.Signal allocates with calloc and realloc instead of using the gc directly? When digging through the code everything seems magic and old.

E.g.: https://github.com/D-Programming-Language/phobos/blob/master/std/signals.d#L199

This looks like an old label used for a goto.

(calloc/realloc:
https://github.com/D-Programming-Language/phobos/blob/master/std/signals.d#L170)

Also rt_detachDisposeEvent and co. are magic to me, I can only guess what they do when digging through the code, what prevents std.signals from using the GC. It's just a guess but, I think this would also allow the use of struct methods as callbacks.

I ran into this issue today:

---
struct Foo {
    Bar bar;

    void my_callback(int i) {
        bar.do_something();
    }
}

slot.connect(&foo.my_callback);
---

This kept segfaulting, chaning Foo to a class solves the issue. (I had more such strange moments, using curry! together with a helper function which bound the callbacks also introduced segfaults).

I think that has to do with the manual memory managment used together with the more or less magic functions.

If you tell me that the manual memory management is an old D1 relict and isn't needed at all, I could clean that code up and submit a pull request (I really learned to like std.signals).

On 1/17/13, David <d@dav1d.de> wrote:
> Is there any reason why std.signals.Signal allocates with calloc and realloc instead of using the gc directly? When digging through the code everything seems magic and old.

I think it's because it's old. It should probably use Appender instead of calloc.

FWIW Robert proposed a new std.signals: http://forum.dlang.org/thread/mailman.1551.1352122526.5162.digitalmars-d@puremagic.com http://forum.dlang.org/thread/mailman.2206.1353757059.5162.digitalmars-d@puremagic.com

Actually it is to emulate weak references. Allocating the memory with malloc is an easy way to have references ignored by the GC. I have already written an improved version of std.signals. It already works, but the template mixin frontend for easy integration into a class (with private signal emission) is currently blocked by bug:

http://d.puremagic.com/issues/show_bug.cgi?id=8441

You can find my implementation at: https://github.com/eskimor/phobos/blob/new_signal/std/signals.d

The current std.signals implementation does not support struct methods as slots, only objects. (Because only objects can notify when being deleted. (This is a documented bug, see: http://dlang.org/phobos/std_signals.html)

My new implementation allows delegates of any type with strongConnect() but you will loose weak ref semantics. And you won't even be able to use connect() with anything else than an object, so the new implementation is not just more powerful, but also prevents from wrong use. You are actually a prime example, that documenting such things is not enough ;-)

Best regards,

Robert

On Thu, 2013-01-17 at 23:33 +0100, David wrote:
> Is there any reason why std.signals.Signal allocates with calloc and realloc instead of using the gc directly? When digging through the code everything seems magic and old.
> 
> E.g.: https://github.com/D-Programming-Language/phobos/blob/master/std/signals.d#L199
> 
> This looks like an old label used for a goto.
> 
> (calloc/realloc:
> https://github.com/D-Programming-Language/phobos/blob/master/std/signals.d#L170)
> 
> Also rt_detachDisposeEvent and co. are magic to me, I can only guess what they do when digging through the code, what prevents std.signals from using the GC. It's just a guess but, I think this would also allow the use of struct methods as callbacks.
> 
> I ran into this issue today:
> 
> ---
> struct Foo {
>     Bar bar;
> 
>     void my_callback(int i) {
>         bar.do_something();
>     }
> }
> 
> slot.connect(&foo.my_callback);
> ---
> 
> This kept segfaulting, chaning Foo to a class solves the issue. (I had more such strange moments, using curry! together with a helper function which bound the callbacks also introduced segfaults).
> 
> I think that has to do with the manual memory managment used together with the more or less magic functions.
> 
> If you tell me that the manual memory management is an old D1 relict and isn't needed at all, I could clean that code up and submit a pull request (I really learned to like std.signals).

The magic comes from the fact, that a signal has weak reference semantics, which means if you drop all references to the target object, it won't be kept in memory because of the connected signal, instead the slot gets de-registered automatically.

Surprisingly a lot of people seem to be unaware of this very useful and important characteristic of signals. Probably because of coming from a C#/Java background instead of a C++ (boost, gtk, QT) background, whose signals implementations are all weak ref based.

Best regards,

Robert

No, this would not solve the issue, except you are going to drop weak ref semantics, which would reduce a signal to a simple array of delegates and greatly defeats its usefulness. (To prevent memory leaks, you would have to deregister all your objects from any signals they might be connected to, when done with an object, which is in some aspects worse and guaranteed more cumbersome than manual memory management).

Also see the following announce for the feature set of the new
implementation:
http://forum.dlang.org/thread/mailman.2206.1353757059.5162.digitalmars-d@puremagic.com

If you want to use the new implementation, all you have to do is drop the template mixin at the beginning and use the FullSignal struct directly.

A possible workaround for the bug would be to use a string mixin, but the syntax would become a bit more ugly and we would be locked up with it then, because a change would break compatibility.

Any comments about the implementation are appreciated.

Best regards,

Robert

On Thu, 2013-01-17 at 23:33 +0100, David wrote:
> If you tell me that the manual memory management is an old D1 relict
> and
> isn't needed at all, I could clean that code up and submit a pull
> request (I really learned to like std.signals).

Am 18.01.2013 12:27, schrieb eskimo:
> The magic comes from the fact, that a signal has weak reference semantics, which means if you drop all references to the target object, it won't be kept in memory because of the connected signal, instead the slot gets de-registered automatically.
> 

What about weakref for objects and for nothing else? __traits/std.traits and friends.

Using a class in my case makes not really sense.

Am 18.01.2013 06:29, schrieb Andrej Mitrovic:
> On 1/17/13, David <d@dav1d.de> wrote:
>> Is there any reason why std.signals.Signal allocates with calloc and realloc instead of using the gc directly? When digging through the code everything seems magic and old.
> 
> I think it's because it's old. It should probably use Appender instead of calloc.
> 
> FWIW Robert proposed a new std.signals: http://forum.dlang.org/thread/mailman.1551.1352122526.5162.digitalmars-d@puremagic.com http://forum.dlang.org/thread/mailman.2206.1353757059.5162.digitalmars-d@puremagic.com
> 

Good to know, thanks

The new implementation offers this possibility, with the strongConnect() method. The old implementation wasn't able to do so, because a delegate contains no type information about the context.

Best regards,

Robert
On Sat, 2013-01-19 at 16:28 +0100, David wrote:
> Am 18.01.2013 12:27, schrieb eskimo:
> > The magic comes from the fact, that a signal has weak reference semantics, which means if you drop all references to the target object, it won't be kept in memory because of the connected signal, instead the slot gets de-registered automatically.
> > 
> 
> What about weakref for objects and for nothing else? __traits/std.traits and friends.
> 
> Using a class in my case makes not really sense.

Am 19.01.2013 19:39, schrieb eskimo:
> The new implementation offers this possibility, with the strongConnect() method. The old implementation wasn't able to do so, because a delegate contains no type information about the context.
> 
Any plans of getting this into Phobos as a std.signals replacement? Otherwise using it doesn't make too much sense. (a class is the lesser of the two evils)

Of course. Just at the moment issue: http://d.puremagic.com/issues/show_bug.cgi?id=8441

is holding it back.

It will be probably named std.signals2 or something to maintain backwards compatibility, but yeah I definitely want to get it into phobos. In fact, I don't even have a use for it in my own projects, I just saw the current implementation and thought there must be a better way. So the whole point of implementing it, was to get it into phobos.

I strongly encourage you to use my implementation, because real world testing before becoming a part of the standard library is always a good idea. But keep in mind, that it wasn't formally reviewed yet so your code might break, when being included into phobos. (Stuff might get renamed for example.) On the other hand fixing this issues should not be too much work. So yeah, please try it out, so it will be perfect when included. The feature set and set of improvements, should be convincing anyway, if not I have done something wrong.

As said before, you will have to remove the template mixin at the beginning, (because of the above mentioned bug) and use the struct FullSignal directly.

The split up in FullSignal and RestrictedSignal, was to be able to disallow others from emitting the signal.

I would be glad to answer any questions about its use and improve documentation where necessary.

Best regards,

Robert

On Sat, 2013-01-19 at 22:47 +0100, David wrote:
> Am 19.01.2013 19:39, schrieb eskimo:
> > The new implementation offers this possibility, with the strongConnect() method. The old implementation wasn't able to do so, because a delegate contains no type information about the context.
> > 
> Any plans of getting this into Phobos as a std.signals replacement? Otherwise using it doesn't make too much sense. (a class is the lesser of the two evils)