libhv icon indicating copy to clipboard operation
libhv copied to clipboard

fix(HttpHandler): prevent exceptions from escaping destructors and cleanup

Open lyriccoder opened this issue 5 months ago • 0 comments

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 .

lyriccoder avatar Sep 05 '25 07:09 lyriccoder