potassco / clingo

🤔 A grounder and solver for logic programs.
https://potassco.org/clingo
MIT License
589 stars 79 forks source link

C++ API throws exception from destructor that cannot be caught #479

Closed adaml-ec closed 5 months ago

adaml-ec commented 5 months ago

Example:

#include <stdio.h>
#include <stdlib.h>
#include <clingo.hh>

using namespace Clingo;

Logger logger = [](Clingo::WarningCode, char const *message) {
    std::cerr << message << std::endl;
};

int main(int argc, char **argv)
{
    try {
        Control ctl{{argv+1, size_t(argc-1)}, logger, 20};
        {
            Backend backend = ctl.backend();
            //do something invalid
            std::vector<id_t> elements;
            backend.theory_atom(1, 1, elements);
            std::cerr << "finished using Backend" << std::endl;
        }
    std::cerr << "Backend out of scope" << std::endl;
    } catch (...) {
        std::cerr << "caught exception" << std::endl;
    }
    std::cerr << "end" << std::endl;
}

This prints:

finished using Backend
terminate called after throwing an instance of 'std::logic_error'
  what():  Unknown term '1'
Aborted

I made an intentionally invalid call to the backend. When the Backend goes out of scope its destructor throws an exception, but I'm unable to catch it.

I might be missing something about what I could do differently, but when I search for information related to this I find advice that destructors should not throw exceptions.

adaml-ec commented 5 months ago

gdb showed it going into throw std::logic_error(msg) in this code, called from the Backend destructor:

inline void handle_error(bool ret) {
    if (!ret) {
        char const *msg = clingo_error_message();
        if (msg == nullptr) { msg = "no message"; }
        switch (static_cast<clingo_error_e>(clingo_error_code())) {
            case clingo_error_runtime:   { throw std::runtime_error(msg); }
            case clingo_error_logic:     { throw std::logic_error(msg); }
            case clingo_error_bad_alloc: { throw std::bad_alloc(); }
            case clingo_error_unknown:
            case clingo_error_success:   { throw std::runtime_error(msg); }
        }
    }
}

inline Backend::~Backend() { // NOLINT
    if (backend_ != nullptr) {
        Detail::handle_error(clingo_backend_end(backend_));
    }
}
adaml-ec commented 5 months ago

My colleague @mbalduccini figured out that a cause of the problem is that C+11 defaults all destructors to noexcept(true), and my program can be made to work (throw exception rather than terminating) if clingo.hh is modified to declare noexcept(false) as in:

 ~Backend() noexcept(false);

and also for ~SolveHandle and ~ProgramBuilder for good measure.

As far as I can tell this is still not recommended as it could cause a resource leak, but it's an improvement.

rkaminsk commented 5 months ago

I'll have a look.