[signals] thread-safe default combiner
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 I'm noticing now that boost::last_value isn't thread-safe when used with a non-void return value, since a thread can't in general be sure the signal will have any slots connected to it when it is invoked. Would it be acceptable to make last_value throw an exception on the non-void, no slots case? Throwing an exception seems preferable to the dreaded "undefined behaviour". Or we could use a slightly modified default combiner with a different name. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFFzKtr5vihyNWuA4URAtTdAKCfJQicTdVtf5qaab5M2PCndy6bowCggwKi DnFSpe3zg5DJsKfCsfnOh7Q= =Tppf -----END PGP SIGNATURE-----
On Feb 9, 2007, at 12:12 PM, Frank Mori Hess wrote:
I'm noticing now that boost::last_value isn't thread-safe when used with a non-void return value, since a thread can't in general be sure the signal will have any slots connected to it when it is invoked. Would it be acceptable to make last_value throw an exception on the non-void, no slots case? Throwing an exception seems preferable to the dreaded "undefined behaviour". Or we could use a slightly modified default combiner with a different name.
Throwing an exception is a reasonable solution. Cheers, Doug
Frank Mori Hess wrote:
I'm noticing now that boost::last_value isn't thread-safe when used with a non-void return value, since a thread can't in general be sure the signal will have any slots connected to it when it is invoked. Would it be acceptable to make last_value throw an exception on the non-void, no slots case? Throwing an exception seems preferable to the dreaded "undefined behaviour". Or we could use a slightly modified default combiner with a different name.
Returning an Optional if T is default constructible seems like a viable refinement of that solution. I'm a little uncomfortable with exceptions being thrown potentially everywhere along the call chain as a default behaviour. Regards Timmo Stange
On Feb 9, 2007, at 3:21 PM, Timmo Stange wrote:
Frank Mori Hess wrote:
I'm noticing now that boost::last_value isn't thread-safe when used with a non-void return value, since a thread can't in general be sure the signal will have any slots connected to it when it is invoked. Would it be acceptable to make last_value throw an exception on the non-void, no slots case? Throwing an exception seems preferable to the dreaded "undefined behaviour". Or we could use a slightly modified default combiner with a different name.
Returning an Optional if T is default constructible seems like a viable refinement of that solution. I'm a little uncomfortable with exceptions being thrown potentially everywhere along the call chain as a default behaviour.
The problem is that we don't get to return an optional, because the return type is specified as "T", not "optional<T>". And, unfortunately, we can't rely on there always being a default constructor there. Right now, the implementation is allowed to crash if you call one of these signals and there are no slots. Throwing an exception brings some sane defined behavior to something that is now completely undefined. If we could reliably detect default-constructibility, we might be able to come up with something better. Cheers, Doug
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Friday 09 February 2007 16:18 pm, Doug Gregor wrote:
Returning an Optional if T is default constructible seems like a viable refinement of that solution. I'm a little uncomfortable with exceptions being thrown potentially everywhere along the call chain as a default behaviour.
The problem is that we don't get to return an optional, because the return type is specified as "T", not "optional<T>". And, unfortunately, we can't rely on there always being a default constructor there.
Right now, the implementation is allowed to crash if you call one of these signals and there are no slots. Throwing an exception brings some sane defined behavior to something that is now completely undefined.
If we could reliably detect default-constructibility, we might be able to come up with something better.
We could add a little header with a definition of a "last_optional" combiner for people to use if they prefer. - -- Frank -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.1 (GNU/Linux) iD8DBQFFzOsr5vihyNWuA4URAnwVAJwJWtZucjcUEc/wBtMA6yYqGqhYVQCdGt+I AKtspvUv6bJM9N1zmoFpCuk= =m/Kn -----END PGP SIGNATURE-----
Frank Mori Hess wrote:
The problem is that we don't get to return an optional, because the return type is specified as "T", not "optional<T>". And, unfortunately, we can't rely on there always being a default constructor there.
Right now, the implementation is allowed to crash if you call one of these signals and there are no slots. Throwing an exception brings some sane defined behavior to something that is now completely undefined.
If we could reliably detect default-constructibility, we might be able to come up with something better.
We could add a little header with a definition of a "last_optional" combiner for people to use if they prefer.
Yes, that'd be nice. I think a non-throwing partial specialization (or a simulated one for compilers that don't support it) of last_value would be good. I've come across another problem that I find difficult to solve: Since signals are not copyable, the current implementation reference-wraps them when they're used as a slot target. The proper lifetime-dependency is established by signals being trackable subclasses, but we agreed that this won't work with thread-safe signals. Our weak_ptr-based solution doesn't work well when the tracked object is the actual target function object. Any suggestions? Regards Timmo Stange
Timmo Stange wrote:
I've come across another problem that I find difficult to solve: Since signals are not copyable, the current implementation reference-wraps them when they're used as a slot target. The proper lifetime-dependency is established by signals being trackable subclasses, but we agreed that this won't work with thread-safe signals. Our weak_ptr-based solution doesn't work well when the tracked object is the actual target function object. Any suggestions?
I think I might have found a solution, if not a particularly beautiful one: With "signal for a slot" being handled as a special case anyway, I can create a "counterfeit copy" of the signal with the original signal object controlling the state and the counterfeit just sharing the implementation. When the original copy goes out of scope, all slots will be disconnected. The rest can be handled with some additional functionality in the tracking code. The series of problems does not end, though ;). Now I have a problem with understanding this in the original code: template<typename T> void maybe_get_pointer(const T& t, mpl::bool_<false>) const { // Take the address of this object, because the object // itself may be trackable add_if_trackable(boost::addressof(t)); } If I'm not mistaken, this takes the address of an object stored by value in the target function object and adds it to the list of tracked objects if it's derived from trackable. What I don't understand is how that is supposed to work. If not provided by reference, the visitor is run over a temporary target function object passed by the client when connecting or creating the slot. It is copied after that by the Function object by default. The tracked object will then be destroyed together with the temporary function object and the slot will be disconnected. The trackable_test.cpp only checks a case in which a pointer to trackable is bound to the function. So is this a bug or am I missing something? Regards Timmo Stange
On Saturday 10 February 2007 01:48 am, Timmo Stange wrote:
Timmo Stange wrote:
I've come across another problem that I find difficult to solve: Since signals are not copyable, the current implementation reference-wraps them when they're used as a slot target. The proper lifetime-dependency is established by signals being trackable subclasses, but we agreed that this won't work with thread-safe signals. Our weak_ptr-based solution doesn't work well when the tracked object is the actual target function object. Any suggestions?
I think I might have found a solution, if not a particularly beautiful one: With "signal for a slot" being handled as a special case anyway, I can create a "counterfeit copy" of the signal with the original signal object controlling the state and the counterfeit just sharing the implementation. When the original copy goes out of scope, all slots will be disconnected. The rest can be handled with some additional functionality in the tracking code.
If the signal is implemented with a pimpl via a shared_ptr, couldn't boost::track just store a weak_ptr to the pimpl in the special case of a signal argument? The signal could invoke the the boost::tracked by using the get_pointer() function and invoking it as a pointer to a function object (assuming the pimpl provides an operator() member function). Is that is what you are already suggesting? -- Frank
Frank Mori Hess wrote:
I think I might have found a solution, if not a particularly beautiful one: With "signal for a slot" being handled as a special case anyway, I can create a "counterfeit copy" of the signal with the original signal object controlling the state and the counterfeit just sharing the implementation. When the original copy goes out of scope, all slots will be disconnected. The rest can be handled with some additional functionality in the tracking code.
If the signal is implemented with a pimpl via a shared_ptr, couldn't boost::track just store a weak_ptr to the pimpl in the special case of a signal argument? The signal could invoke the the boost::tracked by using the get_pointer() function and invoking it as a pointer to a function object (assuming the pimpl provides an operator() member function). Is that is what you are already suggesting?
Our influence on slot target storage and invocation is limited, as the client tells us what container type to use through a template parameter (SlotFunction). I would prefer to store a normal function object there instead of trying to trick it into invoking the object through a pointer and still getting the argument binding right. That's why I'd rather use a copy of the full signal object instead of its pointer to implementation. We can be sure that everything will work as expected, no matter what type SlotFunction is. The tracking will then need extra functionality to identify signal targets that have been disconnected, but I think that's easier to achieve, because we are in control of all the types involved. Regards Timmo Stange
On Saturday 10 February 2007 07:33 pm, Timmo Stange wrote:
If the signal is implemented with a pimpl via a shared_ptr, couldn't boost::track just store a weak_ptr to the pimpl in the special case of a signal argument? The signal could invoke the the boost::tracked by using the get_pointer() function and invoking it as a pointer to a function object (assuming the pimpl provides an operator() member function). Is that is what you are already suggesting?
Our influence on slot target storage and invocation is limited, as the client tells us what container type to use through a template parameter (SlotFunction). I would prefer to store a normal function object there instead of trying to trick it into invoking the object through a pointer and still getting the argument binding right.
Okay, so how about l slightly modify my suggestion to be: store the slot signal as a trivial wrapper function object that holds a weak_ptr to the signal's pimpl, and has an operator() function corresponding to signal invocation. Is that essentially your plan? -- Frank
Frank Mori Hess wrote:
Okay, so how about l slightly modify my suggestion to be: store the slot signal as a trivial wrapper function object that holds a weak_ptr to the signal's pimpl, and has an operator() function corresponding to signal invocation. Is that essentially your plan?
Yes, the signal classes in the original implementation are basically a thin wrapper around it. I suggest to add something like a bool "genuine" as a member, provide a protected copy constructor that initializes it to false (while the public constructor sets it to true) and work with full copies of the signal object. The destructor can disconnect all slots when "genuine" is true. Regards Timmo Stange
On Friday 09 February 2007 08:55 pm, Timmo Stange wrote:
Frank Mori Hess wrote:
The problem is that we don't get to return an optional, because the return type is specified as "T", not "optional<T>". And, unfortunately, we can't rely on there always being a default constructor there.
Right now, the implementation is allowed to crash if you call one of these signals and there are no slots. Throwing an exception brings some sane defined behavior to something that is now completely undefined.
If we could reliably detect default-constructibility, we might be able to come up with something better.
We could add a little header with a definition of a "last_optional" combiner for people to use if they prefer.
Yes, that'd be nice. I think a non-throwing partial specialization (or a simulated one for compilers that don't support it) of last_value would be good.
If I'm following, you're saying something like last_value<T, U=T> would be like the current last_value<T> and last_value<T, optional<T> > would be like last_optional<T>? It seems easier just to provide a stand-alone last_optional<T> since the implementation of last_value<T> is so trivial. -- Frank
Frank Mori Hess wrote:
Yes, that'd be nice. I think a non-throwing partial specialization (or a simulated one for compilers that don't support it) of last_value would be good.
If I'm following, you're saying something like
last_value<T, U=T> would be like the current last_value<T> and last_value<T, optional<T> > would be like last_optional<T>? It seems easier just to provide a stand-alone last_optional<T> since the implementation of last_value<T> is so trivial.
No, I meant a partial specialization for optional<T> directly, i.e. last_value<optional<T> >. That makes the return value of that function object (in our case the combiner) "optional", which reflects the intention to have it not throwing for an empty input range pretty well, I think. Regards Timmo Stange
On Saturday 10 February 2007 07:11 pm, Timmo Stange wrote:
Frank Mori Hess wrote:
Yes, that'd be nice. I think a non-throwing partial specialization (or a simulated one for compilers that don't support it) of last_value would be good.
If I'm following, you're saying something like
last_value<T, U=T> would be like the current last_value<T> and last_value<T, optional<T> > would be like last_optional<T>? It seems easier just to provide a stand-alone last_optional<T> since the implementation of last_value<T> is so trivial.
No, I meant a partial specialization for optional<T> directly, i.e. last_value<optional<T> >. That makes the return value of that function object (in our case the combiner) "optional", which reflects the intention to have it not throwing for an empty input range pretty well, I think.
The work-around for compilers lacking partial specialization is to give the specialization a different class name, right (e.g. last_optional<T>)? So providing a partial specialization would result in last_value<optional<T>
throwing when compiled on some compilers and not on others. It seems less confusing to provide last_optional<T> only.
-- Frank
Frank Mori Hess wrote:
No, I meant a partial specialization for optional<T> directly, i.e. last_value<optional<T> >. That makes the return value of that function object (in our case the combiner) "optional", which reflects the intention to have it not throwing for an empty input range pretty well, I think.
The work-around for compilers lacking partial specialization is to give the specialization a different class name, right (e.g. last_optional<T>)? So providing a partial specialization would result in last_value<optional<T>
throwing when compiled on some compilers and not on others. It seems less confusing to provide last_optional<T> only.
You can use last_value<optional<T> > on compilers that don't have partial template specialization as well, the implementation is just not as straightforward, but that won't be visible to the user. Regards Timmo Stange
On Sunday 11 February 2007 05:02 pm, Timmo Stange wrote:
You can use last_value<optional<T> > on compilers that don't have partial template specialization as well, the implementation is just not as straightforward, but that won't be visible to the user.
Allright, I think I've got it now. I've added a last_value.hpp to the boost sandbox which doesn't throw on void or optional<T> return values, and throws otherwise. -- Frank
participants (4)
-
Doug Gregor -
Frank Mori Hess -
Frank Mori Hess -
Timmo Stange