2

In my project I have a system of events. You can connect a callback to an event and any time the event is sent, your callback(s) is called.

Upon connecting to an event you get a token. As long as the token is not destroyed, the connection is active:

class A
{

    A()
    {
        event_connection = get_dispatcher().connect(event, std::bind(member_function, this));
    }
    void member_function()
    {
        dummy_instance++; //any action that uses this field
    }
    // changed from shared to unique to avoid confusion
    //std::shared_ptr<event_connection_token> event_connection;
    std::unique_ptr<event_connection_token> event_connection;
    dummy_type dummy_instance;
}

However, a problem arises in the following scenario:

  1. Deconstruction of class A starts
  2. Field dummy_instance is destroyed
  3. Now the event occurs
  4. The callback is called because event_connection wasn’t destroyed yet
  5. The callback tries to access the deallocated memory and the program crashes

Therefore, I need my event_connection_token to always be destroyed before any class members that callback is using. Now if I want 100 other programmers to use this event-callback system it would be unprofessional to expect them to always deallocate event_connection_token first in all classes they ever make. We finally come to the question:

How can I enforce that every client removes event_connection_token before anything else in client class gets destroyed?

I’m looking for either:

  • a clever design that will make sure the token is always removed first without programmers even thinking about it, or
  • a compile-time / run-time check that will let programmers know that they need to modify code so that the token is removed first.

EDIT: The question marked as duplicate does not address my problem. I know the order of destruction of objects, or even explicitly calling .reset() in the destructor will fix my problem. That is however not the solution to my problem. The problem is I don’t want to rely on every developer in the project remembering this rule (as this event-callback system is to be used in many places in the code).

Sebastian Simon
  • 18,263
  • 7
  • 55
  • 75
Marcin K.
  • 683
  • 1
  • 9
  • 20
  • @e by πάντα ῥεῖ Unless I misunderstand the question, I don't think it is a duplicate. Here, the OP is using a `std::shared_ptr` so the normal “order of destruction” rules will only ensure that the *handle* is destructed at the “right” time. Whether or not the *managed object* is destructed (at all) is out of the class' control. – 5gon12eder Aug 13 '15 at 16:40
  • Sorry for confusion. It is in fact shared_ptr in my implementation for unrelated reason. For the sake of the example I will change it to unique ptr. It is however NOT duplicated with the question that πάντα ῥεῖ linked. I want to make sure every programmer in the project closes connection before other members of his class get deleted, without the need to inform everyone and expect them to remember this rule (as in "he cant remove anything before connection_token even if he WANTED to"). – Marcin K. Aug 13 '15 at 17:07
  • Then I don't understand your problem. If your class holds a `std::unique_ptr` to the `event_connection`, how can anybody else interfere with it? (That is, unless you start handing out non-owning raw pointers.) – 5gon12eder Aug 13 '15 at 17:11
  • As i said. Im not the only person using this event-callback system. That's not a good solution to just tell 100 people "ok guys remember to always destroy connection before other class members" – Marcin K. Aug 13 '15 at 17:14
  • If you have a `std::unique_ptr` that manages the object, neither you nor your fellow other people need to remember to destroy it. It will just be done. – 5gon12eder Aug 13 '15 at 17:15
  • It sounds like there are threads involved? If so, you need to detail how these threads communicate or signal to each other and you'll need to define more clearly who owns whom. – Niall Aug 13 '15 at 21:31
  • I edited your question slightly to improve the presentation. However, I am slightly confused by the paragraph immediately after your hypothetical scenario. Could you try to clarify it somewhat? Also, I stupidly failed to correct the formatting of the list. –  Aug 14 '15 at 01:06
  • Look at how Qt handles this. If you want to be cleaver and expect things to run in multithreaded way, then you have a problem as you'll always have race conditions like you illustrated. – user3427419 Aug 14 '15 at 01:26
  • use locks, semaphores, .... ? – bolov Aug 14 '15 at 01:35
  • I think i made a mistake by putting a code that USES the event-callback system and not the implementation of event-callback system itself. I will edit the question soon with how is event-callback system implemented. – Marcin K. Aug 17 '15 at 09:58

2 Answers2

1

Just swap the declarations

class A
{

    A()
    {
        event_connection = get_dispatcher().connect(event, std::bind(member_function, this));
    }
    void member_function()
    {
        dummy_instance++; //any action that uses this field
    }
    // changed from shared to unique to avoid confusion
    //std::shared_ptr<event_connection_token> event_connection;

    dummy_type dummy_instance;
    std::unique_ptr<event_connection_token> event_connection;
}

the destruction order is in reverse declaration order (because construction happens in declaration order). When destroying an instance now, first the destructor of the instance is called, then event_connection is destroyed an last dummy_instance is destroyed (construction happens in the reverse order).

I think you'll have to live with the fact that there are some rules that has to be obeyed in order to guarantee this if you don't want to go far in preventing them to do "stupid" things (and I don't even think that you can cover all corner cases anyway).

If you cannot require them to put event_connection last, then you have to forbid them from adding it via composition (and even then if you add it at all you'll end up requiring them to explicitely delete the pointer). This would rule out having event_connection in A in the first place, but rather only allow event_connection to have a reference to A, which would work well if you use smart pointer (except that this would mean that the A object would remain as long as event_connection remains).

skyking
  • 13,817
  • 1
  • 35
  • 57
  • 1
    He explicitly said _"I know the order of destruction of objects (...) will fix my problem. That is however not the solution to my problem"_... – Tomasz Sodzawiczny Aug 14 '15 at 12:46
0

You could try excluding the actual callback implementation to a separate class, and then compose it into a "keeper" class:

class ACallback
{
    public:
        void member_function() 
        {
            dummy_instance++; //any action that uses this field
        }
    private:
        dummy_type dummy_instance;
}

class A 
{
    A(ACallback *callback) : callback(callback)
    {
        event_connection = get_dispatcher().connect(event, std::bind(ACallback::member_function, callback));
    }
    ~A()
    {
        // make sure callback will not be used any more
    }
    std::unique_ptr<ACallback> callback;
    std::unique_ptr<event_connection_token> event_connection;
}