When RAII is not an option, how to ensure a function is called before end of scope of an object?

1.1k Views Asked by At

The problem is as following: we have a database of many different values. Many different classes can modify values in it, and we have to emit an event on any modification, so that other classes can be notified about modification. We currently have wrappers like this (see it online):

struct Database {
    std::string s;
    int i;
};

template <auto PMD>
struct DbFieldAccessor {
    // template magic, from (PMD = T C::*) extracts T
    using FieldType = PmdFieldType_t<decltype(PMD)>;

    explicit DbFieldAccessor(Database& database) : m_database{&database} {}

    constexpr auto operator->() noexcept -> FieldType* {
        m_modified = true;
        return &(m_database->*PMD);
    }

    auto isFieldModified() const noexcept -> bool { return m_modified; }

   private:
    Database* m_database;
    bool m_modified = false;
};

template <class Db>
class DatabaseAdapter {
   public:
    explicit DatabaseAdapter(Db db) : m_database{std::move(db)} {}

    template <auto PMD>
    [[nodiscard]] auto mutableAccess() -> DbFieldAccessor<PMD> {
        return DbFieldAccessor<PMD>{m_database};
    }
    template <auto... PMDs>
    auto emitUpdates(const DbFieldAccessor<PMDs>&... accessors) -> void {
        (
            [](const auto& accessor) {
                if (accessor.isFieldModified()) {
                    // emitEvent(events::FieldUpdated<PMDs>{});
                }
            }(accessors),
            ...);
    }

   private:
    Db m_database{};
};

The expected usage is like this:

    Database internalDb; // never accessed directly
    DatabaseAdapter<Database> db{internalDb};

    auto access = db.mutableAccess<&Database::s>(); // get access to s in Database
    access->push_back('D'); // modify it
    db.emitUpdates(access); // emit updates

The problem is that it's easy to forget to call emitUpdates after modifying a field. We've been trying to find a solution that calls it automagically, but couldn't come with anything:

  1. RAII - calling emitUpdates in ~DbFieldAccessor cannot be done, emitUpdates can throw
  2. wrapping it in a function - creates unnecessary copies and is sometimes inconvenient (see above example - instead of push_back, we need to pass getFromDb(s) + 'D')
template<auto PMD, class Db>
void modifyDb(DatabaseAdapter<Db>& db, const PmdFieldType_t<decltype(PMD)>& data)
{
    auto access = db.template mutableAccess<PMD>();
    *access = data;
    db.emitUpdates(dispatcher, entityId, access);
}
  1. pass a lambda to modify database member - there's some boilerplate at call site and if we lazily pass auto& as lambda argument, we lose any code completion features
template<auto PMD, class Db, class Func>
void modifyDb(DatabaseAdapter<Db>& db, Func&& func)
{
    auto access = db.template mutableAccess<PMD>();
    std::forward<Func>(func)(*access);
    db.emitUpdates(dispatcher, entityId, access);
}
// at call site
modifyDb<&Database::s>(db, [](auto& s){s.push_back('D');});

Is there anything else that could be used here? All the code belongs to us, so no problem with any modifications.

4

There are 4 best solutions below

1
463035818_is_not_an_ai On

If you can handle the exception that is potentially thrown by emitUpdates in the code that calls emitUpdates then you can still make use of RAII:

#include <iostream>
#include <stdexcept>


template <typename F,typename Handler>
struct raii {
    F f;
    Handler handler;
    raii(const F& f,const Handler& handler) : f(f),handler(handler) {} 
    ~raii() {
        try {
            f();
        } catch (std::exception& e){
            handler(e);
        } catch (...) {
            // necessary to make sure no exception leaves the destructor
        }
    }
};

void foo() {
    raii r{[](){ throw std::runtime_error("boom");},
           [](std::exception& e) { std::cout << e.what();}};
    std::cout << "hey\n";
}

int main() {
    foo();
}

Live Demo

As pointed out in comments, if you need the exception to travel further up the call stack before it can be handled then the above is not viable.

3
Botje On

Clang has experimental typestate annotations you could abuse for this. They are meant to track the "consumed" state of resources, in this case your DbFieldAccessor objects start in the unconsumed state when created and the emitUpdates method consumes the obligation.

Just pasting in the two changed objects:

template <auto PMD>
struct [[clang::consumable(unconsumed)]]DbFieldAccessor {
    // template magic, from PMD = T C::* extracts T
    using FieldType = PmdFieldType_t<decltype(PMD)>;

    [[clang::return_typestate(unconsumed)]]
    explicit DbFieldAccessor(Database& database) : m_database{&database} {}

    [[clang::callable_when(consumed)]]
    ~DbFieldAccessor() {}
    constexpr auto operator->() noexcept -> FieldType* {
        m_modified = true;
        return &(m_database->*PMD);
    }

    auto isFieldModified() const noexcept -> bool { return m_modified; }
    [[clang::set_typestate(consumed)]]
    void assertConsumed() const {}
   private:
    Database* m_database;
    bool m_modified = false;
};

template <class Db>
class  DatabaseAdapter {
   public:
    explicit DatabaseAdapter(Db db) : m_database{std::move(db)} {}
    template <auto PMD>
    [[nodiscard]] auto mutableAccess() -> DbFieldAccessor<PMD> {
        return DbFieldAccessor<PMD>{m_database};
    }
    template <auto... PMDs>
    auto emitUpdates( [[clang::return_typestate(consumed)]] const DbFieldAccessor<PMDs>&... accessors) -> void {
        (
            []([[clang::return_typestate(consumed)]] const auto& accessor) {
                if (accessor.isFieldModified()) {
                    // emitEvent(events::FieldUpdated<PMDs>{});
                }
                accessor.assertConsumed();
            }(accessors),
            ...);
    }

   private:
    Db m_database{};
};

For the following main function:

int main() {
    Database internalDb;
    DatabaseAdapter<Database> db{internalDb};

    auto access = db.mutableAccess<&Database::s>();
    access->push_back('D');

    auto access2 = db.mutableAccess<&Database::i>();
    
    db.emitUpdates(access);
}

I receive the following warning because access2 is not being consumed, while access is okay.

<source>:83:1: warning: invalid invocation of method '~DbFieldAccessor' on object 'access2' while it is in the 'unconsumed' state [-Wconsumed]

And of course I get two errors if I emit the db.emitUpdates call altogether:

<source>:83:1: warning: invalid invocation of method '~DbFieldAccessor' on object 'access2' while it is in the 'unconsumed' state [-Wconsumed].
<source>:83:1: warning: invalid invocation of method '~DbFieldAccessor' on object 'access' while it is in the 'unconsumed' state [-Wconsumed]

2
user7860670 On

One approach to this sort of "dont't forget to call at the end" problems is to design a sort of stream API where result of each operation, except for the last one, can not be discarded:

class Access
{
    public: [[nodiscard("Invoke .done() as a last operation!")]] Access &
    push_back(char)
    {
        // modify...
        return *this;
    }

    public: void
    done()
    {
        // emitUpdates()...
    }
};

int main()
{
    Access access{};
    access.push_back('a').push_back('b').done();// OK
    access.push_back('a').push_back('b');// 'Invoke .done() as a last operation!' [-Wunused-result]
}

online compiler

0
Cort Ammon On

In this situation you should think very carefully about what makes sense. You mention this issue with using RAII:

RAII - calling emitUpdates in ~DbFieldAccessor cannot be done, emitUpdates can throw

This tells me two key things about your code:

  • You can throw exceptions
  • emitUpdates can fail, and you rely on exceptional control flow to handle the case when emitUpdates fails.

Now the key question here is "what is an acceptable behavior if emitUpdates fails?" And this is asked in the context of an unspecified exceptional control flow from the user functionality. Is there a safe way to roll back the updates? If so, no exception is needed from emitUpdates.

From the fact that you felt the need to throw exceptions from emitUpdates, that suggests you don't have a safe way to deal with these exceptions.

Fundamentally, you are looking for a solution that interacts with the exception handling mechanism. This means you are either using try/catch or RAII. If you are worried about throwing exceptions from destructors, you have to ask yourself what the try/catch code would look like. If your try/catch code fails (emitUpdates fails), what is your plan to handle that case?

If there's an acceptable way to handle this case, but you want to throw an exception anyway, consider using std::current_exception in the destructor to determine whether one is in the process of unwinding, and only throw if one is not unwinding (falling back on the no-throw behavior only when necessary)

If there is no acceptable way to handle this case, then the issue may not be an RAII issue but a fundamental design issue. You may need to apply rules elsewhere to have the user be able to make a conscious choice about how to propagate exceptions.

Such design topics are far beyond the scope of SE, but there are some solutions. One solution is to make this destructor safe by adding checking into other operations. If one can construct the datasturctures to permit an "invalidated" bit, other activities can check this bit before proceeding.

In such a pattern, the documentation might state that if emitUpdates would otherwise throw an exception but an exception is currently being propagated, it instead invalidates the adapter. All future calls to the adapter or its related classes will throw a DatabaseInvalidated exception.