You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
As @MCFlowMace mentioned in #91, we should probably not be passing std::shared_ptrs in add_cancelable() and remove_cancelable(). #91 (comment)
We should consider how signal_handler uses references to cancelables to decide what the right pointer type is for the function interface and for the storing. Let's use this thread to figure out what the right path is.
Considerations
For the most part, signal_handler should not own the cancelables, and should not control their lifetime or behavior with one exception: at some point determined by the user or client code, signal_handler will tell the cancelables it knows about and are valid to cancel their operations.
Not all cancelables are passed to signal_handler for cancelation responsibilities. Therefore cancelable itself cannot assume that it gets added to or removed from signal_handler at any given times (e.g. at construction and destruction, respectively).
Therefore signal_handler needs a way to store a non-owning reference to a subset of the cancelables that exist in an application at any given time. At some point signal_handler will need to use those references without dereferencing pointers to objects that have already been deleted.
Current reference storage
My solution to the storage question was to use weak_ptr. Actually, the reference are stored in a map< cancelable*, weak_ptr<cancelable> > since sets cannot be indexed by std::weak_ptrs. When signal_handler::cancel_all() is used, `weak_ptr::expired() is used to check whether the pointed-to object still exists before the pointer is dereferenced.
Current interface
Right now the add_cancelable() and remove_cancelable() functions take shared_ptr<cancelable> as their argument.
Suggestion
If the storage solution seems reasonable, then the main thing that probably needs changing is the interface arguments. They. shouldn't use shared_ptr because of the reasons @MCFlowMace mentioned. What if instead we pass weak_ptr<cancelable>? This would not imply ownership, and it would still pass the needed information.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
As @MCFlowMace mentioned in #91, we should probably not be passing
std::shared_ptr
s inadd_cancelable()
andremove_cancelable()
. #91 (comment)We should consider how
signal_handler
uses references tocancelable
s to decide what the right pointer type is for the function interface and for the storing. Let's use this thread to figure out what the right path is.Considerations
For the most part,
signal_handler
should not own thecancelable
s, and should not control their lifetime or behavior with one exception: at some point determined by the user or client code,signal_handler
will tell thecancelable
s it knows about and are valid to cancel their operations.Not all
cancelable
s are passed tosignal_handler
for cancelation responsibilities. Thereforecancelable
itself cannot assume that it gets added to or removed fromsignal_handler
at any given times (e.g. at construction and destruction, respectively).Therefore
signal_handler
needs a way to store a non-owning reference to a subset of thecancelable
s that exist in an application at any given time. At some pointsignal_handler
will need to use those references without dereferencing pointers to objects that have already been deleted.Current reference storage
My solution to the storage question was to use
weak_ptr
. Actually, the reference are stored in amap< cancelable*, weak_ptr<cancelable> >
since sets cannot be indexed bystd::weak_ptr
s. Whensignal_handler::cancel_all()
is used, `weak_ptr::expired() is used to check whether the pointed-to object still exists before the pointer is dereferenced.Current interface
Right now the
add_cancelable()
andremove_cancelable()
functions takeshared_ptr<cancelable>
as their argument.Suggestion
If the storage solution seems reasonable, then the main thing that probably needs changing is the interface arguments. They. shouldn't use
shared_ptr
because of the reasons @MCFlowMace mentioned. What if instead we passweak_ptr<cancelable>
? This would not imply ownership, and it would still pass the needed information.Beta Was this translation helpful? Give feedback.
All reactions