Skip to content

Conversation

lyriccoder
Copy link

Problem:
Currently, ~HttpHandler(), Close(), killTimer(), and erase() can propagate exceptions through destructors or cleanup code.
This violates safe C++ practices because throwing exceptions from destructors is undefined behavior if another exception is already active (stack unwinding). In addition, placement new or STL container operations (like std::deque::push_back) can throw, which might escape through the destructor.

This is flagged by static analyzers and linters as a critical issue: destructors must not throw. Even if the code works most of the time, a single allocation failure or exception in a callback can terminate the program unexpectedly.

Potential call stack where exceptions can propagate:

~HttpHandler()
 └─ Close()
     └─ closeFile()
         └─ killTimer()
             └─ runInLoop(lambda)
                 └─ queueInLoop()
                     └─ postEvent()
                         └─ customEvents.push(ev)  <-- std::length_error or other exceptions

There are 3 potential fixes, (I am suggesting the first one):

  1. ~HttpHandler() now wraps Close() in a try/catch to silently swallow any exceptions.
  2. Close() and killTimer() are marked noexcept and all potentially throwing operations are wrapped in try/catch.
void HttpHandler::killTimer(TimerID timerID) noexcept {
    runInLoop([timerID, this]() noexcept {
        try {
            auto iter = timers.find(timerID);
            if (iter != timers.end()) {
                htimer_del(iter->second->timer);
                timers.erase(iter);
            }
        } catch (...) {
            // exceptions swallowed safely
        }
    });
}
  1. Replace placement new with move assignment, which is noexcept if move constructor is noexcept.
    Guarantees no exceptions propagate from erase.
size_type erase(const Key& key) {
    for (auto it = this->begin(); it != this->end(); ++it) {
        if (it->first == key) {
            for (auto moveIt = it; moveIt + 1 != this->end(); ++moveIt) {
                *moveIt = std::move(*(moveIt + 1));
            }
            Container::pop_back();
            return 1;
        }
    }
    return 0;
}

This guarantees that destructors and cleanup routines cannot throw, satisfying the C++ Core Guidelines rule: C.64: Destructors should be noexcept
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant