Copilot commented on code in PR #59769:
URL: https://github.com/apache/doris/pull/59769#discussion_r2681307939
##########
be/src/http/ev_http_server.cpp:
##########
@@ -136,22 +134,49 @@ void EvHttpServer::start() {
evhttp_set_newreqcb(http.get(), on_connection, this);
evhttp_set_gencb(http.get(), on_request, this);
+ _evhttp_servers[i] = http;
+ }
+ }
+
+ for (int i = 0; i < _num_workers; ++i) {
+ auto status = _workers->submit_func([this, i]() {
+ std::shared_ptr<event_base> base;
+ {
+ std::lock_guard lock(_event_bases_lock);
+ base = _event_bases[i];
+ }
event_base_dispatch(base.get());
});
CHECK(status.ok());
}
}
void EvHttpServer::stop() {
+ // 1. Close server fd first to reject new connections
+ close(_server_fd);
+ _server_fd = -1;
+
+ // 2. Break all event loops to make dispatch return
{
std::lock_guard<std::mutex> lock(_event_bases_lock);
for (int i = 0; i < _num_workers; ++i) {
- event_base_loopbreak(_event_bases[i].get());
+ if (_event_bases[i]) {
+ event_base_loopbreak(_event_bases[i].get());
+ }
}
}
+
+ // 3. Wait for all worker threads to finish event_base_dispatch
_workers->shutdown();
Review Comment:
The _workers->shutdown() call should check if _workers is not null before
calling shutdown(). If stop() is called before start() completes or if the
ThreadPoolBuilder fails, _workers could still be null, leading to a null
pointer dereference. Add a check: if (_workers) before calling
_workers->shutdown().
```suggestion
if (_workers) {
_workers->shutdown();
}
```
##########
be/src/http/ev_http_server.cpp:
##########
@@ -136,22 +134,49 @@ void EvHttpServer::start() {
evhttp_set_newreqcb(http.get(), on_connection, this);
evhttp_set_gencb(http.get(), on_request, this);
+ _evhttp_servers[i] = http;
+ }
+ }
+
+ for (int i = 0; i < _num_workers; ++i) {
+ auto status = _workers->submit_func([this, i]() {
+ std::shared_ptr<event_base> base;
+ {
+ std::lock_guard lock(_event_bases_lock);
+ base = _event_bases[i];
+ }
event_base_dispatch(base.get());
});
CHECK(status.ok());
}
}
void EvHttpServer::stop() {
+ // 1. Close server fd first to reject new connections
+ close(_server_fd);
Review Comment:
The close() function should check if _server_fd is valid before attempting
to close it. If stop() is called before start() completes or if _bind() fails,
_server_fd could still be -1, and close(-1) will fail with errno EBADF. Add a
check: if (_server_fd >= 0) before calling close().
```suggestion
if (_server_fd >= 0) {
close(_server_fd);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
