Skip to content

refactor: remove try/catch from event dispatch and IOCP completion handlers#5

Merged
lmshao merged 1 commit into
masterfrom
copilot/analyze-code-defects
May 10, 2026
Merged

refactor: remove try/catch from event dispatch and IOCP completion handlers#5
lmshao merged 1 commit into
masterfrom
copilot/analyze-code-defects

Conversation

Copilot AI commented May 10, 2026

Copy link
Copy Markdown
Contributor

The codebase follows a no-exception convention — handlers communicate errors via return values and LMNET_LOG*, never by throwing. The try/catch blocks in the event reactor dispatch paths and the IOCP worker thread were dead code that added noise and inconsistency.

Changes

  • src/platforms/darwin/event_reactor.cpp — removed try/catch wrapper around DispatchEvent() in the kqueue run loop
  • src/platforms/linux/epoll/event_reactor.cpp — removed try/catch wrapper inside DispatchEvent() around the four handler calls
  • src/platforms/windows/iocp_manager.cpp — removed try/catch wrapper around HandleCompletion() in the IOCP worker thread; recycleRequest is now assigned directly from the return value
// Before
bool recycleRequest = true;
try {
    recycleRequest = HandleCompletion(req, bytes, error);
} catch (const std::exception &e) {
    LMNET_LOGE("Exception in IOCP completion handler: %s", e.what());
} catch (...) {
    LMNET_LOGE("Unknown exception in IOCP completion handler");
}

// After
bool recycleRequest = HandleCompletion(req, bytes, error);

…ndlers

Per project convention, exception-based error handling is not used.
All event handlers (HandleRead/Write/Error/Close) and IOCP completion
callbacks communicate errors via return values and LMNET_LOG*, not
exceptions. Remove the try/catch wrappers from:
- darwin kqueue EventReactor::Run()
- linux epoll EventReactor::DispatchEvent()
- windows IocpManager worker thread

Agent-Logs-Url: https://github.com/lmshao/lmnet/sessions/f7bca67d-7d38-40f2-96d7-6f7703e6565e

Co-authored-by: lmshao <3686112+lmshao@users.noreply.github.com>
@lmshao lmshao marked this pull request as ready for review May 10, 2026 16:17
Copilot AI review requested due to automatic review settings May 10, 2026 16:17
@lmshao lmshao merged commit cd63829 into master May 10, 2026
9 checks passed
@lmshao lmshao deleted the copilot/analyze-code-defects branch May 10, 2026 16:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes try/catch wrappers from the platform-specific event dispatch paths (kqueue/epoll) and the Windows IOCP worker loop to align reactor/IO completion handling with the project’s stated no-exceptions convention.

Changes:

  • Removed try/catch around kqueue dispatch in the Darwin reactor run loop.
  • Removed try/catch around handler invocations in the Linux epoll DispatchEvent() path.
  • Removed try/catch around HandleCompletion() in the Windows IOCP worker thread and assigns recycleRequest directly from the return value.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/platforms/darwin/event_reactor.cpp Removes exception wrapper around kqueue event dispatch.
src/platforms/linux/epoll/event_reactor.cpp Removes exception wrapper around epoll handler calls in DispatchEvent().
src/platforms/windows/iocp_manager.cpp Removes exception wrapper around IOCP completion handling in the worker loop.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 203 to 206
// Handle completion
bool recycleRequest = true;
try {
recycleRequest = HandleCompletion(req, bytes, error);
} catch (const std::exception &e) {
LMNET_LOGE("Exception in IOCP completion handler: %s", e.what());
} catch (...) {
LMNET_LOGE("Unknown exception in IOCP completion handler");
}
bool recycleRequest = HandleCompletion(req, bytes, error);

if (recycleRequest) {
Comment on lines +279 to +283
if (events & EPOLLIN) {
handler->HandleRead(fd);
}

if (events & EPOLLOUT) {
handler->HandleWrite(fd);
}
if (events & EPOLLOUT) {
Comment on lines 138 to 140
if (handler) {
try {
DispatchEvent(std::move(handler), kev);
} catch (const std::exception &e) {
LMNET_LOGE("Exception in event handler for fd %lld: %s", static_cast<long long>(kev.ident),
e.what());
} catch (...) {
LMNET_LOGE("Unknown exception in event handler for fd %lld", static_cast<long long>(kev.ident));
}
DispatchEvent(std::move(handler), kev);
}
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.

3 participants