This code currently fails with a RangeError (used to work in 2.062)

// http://dpaste.dzfl.pl/332a71ec
import std.stdio;
import std.signals;


struct X {
	mixin Signal!();
}

class O {
	void m() {}
}

void main() {
	O o = new O();
	X[string] aa;
	aa["o"] = X.init;
	aa["o"].connect(&o.m);


	/*{ // 20
		X x = aa["o"];
	}*/
}


If you take the uncomment the "block" at line 20 you end up with a segmentation fault, also worked in 2.062. Both times the problem is in the __dtor (segfault happens in the call to "_d_toObject(stor.ptr)".

Changing "struct" to "class" and "X.init" to "new X()" it seems to work
as it should.
Is this worth a bugreport or was the old behaviour never intended?
This bug (I consider it one) broke quite a few lines of code... If the
old behaviour was never intended, why wasn't it documentated then... oh
well I am drifting into another D rant here...

14.06.2013 18:31, David пишет:
> This code currently fails with a RangeError (used to work in 2.062)
>
> // http://dpaste.dzfl.pl/332a71ec
> import std.stdio;
> import std.signals;
>
>
> struct X {
> 	mixin Signal!();
> }
>
> class O {
> 	void m() {}
> }
>
> void main() {
> 	O o = new O();
> 	X[string] aa;
> 	aa["o"] = X.init;
> 	aa["o"].connect(&o.m);
> 	
>
> 	/*{ // 20
> 		X x = aa["o"];
> 	}*/		
> }
>
>
> If you take the uncomment the "block" at line 20 you end up with a
> segmentation fault, also worked in 2.062. Both times the problem is in
> the __dtor (segfault happens in the call to "_d_toObject(stor.ptr)".
>
> Changing "struct" to "class" and "X.init" to "new X()" it seems to work
> as it should.
> Is this worth a bugreport or was the old behaviour never intended?
> This bug (I consider it one) broke quite a few lines of code... If the
> old behaviour was never intended, why wasn't it documentated then... oh
> well I am drifting into another D rant here...
>

http://dlang.org/phobos/std_signals.html#Signal
"Mixin to create a signal within a class object."

So your `X` must be a class.

Also don't use std.signals - it's an incorrect and dangerous mess (see bug tracker).

-- 
Денис В. Шеломовский
Denis V. Shelomovskij

15.06.2013 14:19, David пишет:
>> http://dlang.org/phobos/std_signals.html#Signal
>> "Mixin to create a signal within a class object."
>>
>> So your `X` must be a class.
> Oh right I have totally overseen that, still used to work really well with 2.063
It can work only by accident iff the struct isn't moved in memory no matter what compiler version you use.
>
>> Also don't use std.signals - it's an incorrect and dangerous mess (see
>> bug tracker).
> Provide me something better in phobos and so far it works pretty good (except the update breaking struct-signals)

Currently there is no working signals implementation in Phobos. The worst thing about the current implementation is it will fail you program accidentally in rare cases. Don't use it.

For issues, see: http://d.puremagic.com/issues/show_bug.cgi?id=4150 http://d.puremagic.com/issues/show_bug.cgi?id=9606 http://d.puremagic.com/issues/show_bug.cgi?id=9603


Also post to NG instead of emailing directly to allow others to follow the discussion.

Also if you _do_ need working signals I can help a with implementation.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij

-------------------------------------------------------------------------------------

Regarding E-mailing, sorry about that, I hate the thunderbird answer button....

> It can work only by accident iff the struct isn't moved in memory no >
matter what compiler version you use.

This worked for at least 3 compiler versions now

I am aware of all of these bugs, none are critical if you know them
4150: use only class methods as event handlers
9606: emitting signals from a different thread would segfault in my
application anayways (opengl code)
9603: basically same as 4150

Also wasn't there a std.signals2 somewhere...

I looked into the "new std.signals" today:

https://github.com/eskimor/phobos/blob/new_signal/std/signals.d

It is completly unusable, "mixin Signal!() x" is blocked by bug, it
doesn't compile due to a wrong type (relativly easy fix), then when
using -unittest it fails to compile, compiler also doesn't give any
hints what exactly fails.
Removing these unittests makes it at least compile, but the other
unittests fail (assert). Removing also these unittests (hoping it still
works), nope it doesn't signals aren't triggered at all.

Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(&my_handler) disconnects all, yay what a joy)


I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the "new std.signals" developers.

On Sun, 2013-06-16 at 15:25 +0200, David wrote:
> I looked into the "new std.signals" today:

Thanks for having a look :-)

> 
> https://github.com/eskimor/phobos/blob/new_signal/std/signals.d
> 
> It is completly unusable, "mixin Signal!() x" is blocked by bug, it doesn't compile due to a wrong type (relativly easy fix), then when using -unittest it fails to compile, compiler also doesn't give any hints what exactly fails.

Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally.

> Removing these unittests makes it at least compile, but the other
> unittests fail (assert). Removing also these unittests (hoping it still
> works), nope it doesn't signals aren't triggered at all.
> 
> Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(&my_handler) disconnects all, yay what a joy)

There are plans absolutely, to be concrete: I plan to make it ready in July. There is one blocker currently: http://d.puremagic.com/issues/show_bug.cgi?id=8441

but there is already an aging pull request which fixes it:

https://github.com/D-Programming-Language/dmd/pull/1660 (4 months old
already)

I hope it will be merged soon, then there is hopefully nothing that hinders my plans. The very least it should become ready during summer.

> 
> 
> I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the "new std.signals" developers.

I almost missed it ;-)

Sorry about your bad experiences with the current version.

Best regards,

Robert

>> https://github.com/eskimor/phobos/blob/new_signal/std/signals.d
>>
>> It is completly unusable, "mixin Signal!() x" is blocked by bug, it doesn't compile due to a wrong type (relativly easy fix), then when using -unittest it fails to compile, compiler also doesn't give any hints what exactly fails.
> 
> Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally.

Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though

>> Removing these unittests makes it at least compile, but the other
>> unittests fail (assert). Removing also these unittests (hoping it still
>> works), nope it doesn't signals aren't triggered at all.
>>
>> Are there any plans on improving this situation with std.signals or the new implementation, currently both are getting less useable every day... (e.g. signal.disconnect(&my_handler) disconnects all, yay what a joy)
> 
> There are plans absolutely, to be concrete: I plan to make it ready in July. There is one blocker currently: http://d.puremagic.com/issues/show_bug.cgi?id=8441
> 
> but there is already an aging pull request which fixes it:
> 
> https://github.com/D-Programming-Language/dmd/pull/1660 (4 months old
> already)
> 
> I hope it will be merged soon, then there is hopefully nothing that hinders my plans. The very least it should become ready during summer.

This blocker wasn't a problem, I used the FullSignal, I can live with that, I don't like to have a private emit method anyways (in my opinion, private is always wrong)

>> I am hijacking this thread in hope someone will read that ;) Hopefully also someone of the "new std.signals" developers.
> 
> I almost missed it ;-)
> 
> Sorry about your bad experiences with the current version.

Hehe wouldn't say it is your fault, obviously it worked at some point (I
guess)... I blame DMD, Ranges and the GC of course ;)

- David

> > Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally.
> 
> Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though
It is yes. It is blocked and I got busy with other stuff, so I did put it on hold. Current master never worked, commits prior to (and including):

04c951e34623e9365a3874c89f43eb997a7b376c

should work (if you drop the mixin part). The Heisenbug is still present
though. (That's the reason for the CAS stuff)


> This blocker wasn't a problem, I used the FullSignal, I can live with that, I don't like to have a private emit method anyways (in my opinion, private is always wrong)

Then you might want to try out an older version, see above.

> Hehe wouldn't say it is your fault, obviously it worked at some point (I
> guess)... I blame DMD, Ranges and the GC of course ;)

Well master never worked (it is work in progress), but older versions
did and should still.

Best regards,

Robert

Am 17.06.2013 22:23, schrieb Robert:
>>> Current master is an experimental CAS based implementation- untested and very likely to get completely reworked internally.
>>
>> Good to know, but that is already a few monce old, or? I remember seeing CAS in the code though
> It is yes. It is blocked and I got busy with other stuff, so I did put it on hold. Current master never worked, commits prior to (and including):
> 
> 04c951e34623e9365a3874c89f43eb997a7b376c
> 
> should work (if you drop the mixin part). The Heisenbug is still present
> though. (That's the reason for the CAS stuff)

Thanks, might try again, because for what I wanna do now, I really need strongConnect, so far it wasn't an issue but it's getting one...

I just finished a new implementation, replacing the template mixin with a string mixin. I also changed the name from signals2 to signal. You can find it here:

https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d

All unittests pass, you don't need any patched compiler. I still have to add some more checks and do some polishing, I will also put it in the dub registry. But you seem to have an urgent need, so feel free to try it out immediately - Be my pre-alpha Tester :-)

Best regards,

Robert

Am 12.07.2013 23:47, schrieb Robert:
> I just finished a new implementation, replacing the template mixin with a string mixin. I also changed the name from signals2 to signal. You can find it here:
> 
> https://github.com/phobos-x/phobosx/blob/master/source/phobosx/signal.d
> 
> All unittests pass, you don't need any patched compiler. I still have to add some more checks and do some polishing, I will also put it in the dub registry. But you seem to have an urgent need, so feel free to try it out immediately - Be my pre-alpha Tester :-)
> 
> Best regards,
> 
> Robert
> 
> 

Bad timing, just got "our"[1] own implementation.
If I have time, I'll play around with it! Thanks for the great work,
maybe we can get something working into phobos...


[1]https://github.com/AndrejMitrovic/new_signals