Skip to content

Commit 9a7b01f

Browse files
16bit-ykikoclaude
andcommitted
fix: address CodeRabbit review findings
- Close stderr_fd on spawn failure in worker test helpers to prevent fd leak - Guard send_stateful/send_stateless against empty worker pools to prevent out-of-bounds access when stateful_count or stateless_count is zero - Short-circuit all stateful feature requests when ensure_compiled() returns false (document not found) instead of forwarding to worker Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0bd8242 commit 9a7b01f

File tree

3 files changed

+32
-13
lines changed

3 files changed

+32
-13
lines changed

src/server/master_server.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -625,8 +625,9 @@ void MasterServer::register_handlers() {
625625
auto path = uri_to_path(params.text_document_position_params.text_document.uri);
626626
auto path_id = path_pool.intern(path);
627627

628-
co_await ensure_compiled(path_id,
629-
params.text_document_position_params.text_document.uri);
628+
if(!co_await ensure_compiled(path_id,
629+
params.text_document_position_params.text_document.uri))
630+
co_return serde_raw{"null"};
630631

631632
worker::HoverParams wp;
632633
wp.path = path;
@@ -655,7 +656,8 @@ void MasterServer::register_handlers() {
655656
auto path = uri_to_path(params.text_document.uri);
656657
auto path_id = path_pool.intern(path);
657658

658-
co_await ensure_compiled(path_id, params.text_document.uri);
659+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
660+
co_return serde_raw{"null"};
659661

660662
worker::SemanticTokensParams wp;
661663
wp.path = path;
@@ -676,7 +678,8 @@ void MasterServer::register_handlers() {
676678
auto path = uri_to_path(params.text_document.uri);
677679
auto path_id = path_pool.intern(path);
678680

679-
co_await ensure_compiled(path_id, params.text_document.uri);
681+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
682+
co_return serde_raw{"null"};
680683

681684
worker::InlayHintsParams wp;
682685
wp.path = path;
@@ -697,7 +700,8 @@ void MasterServer::register_handlers() {
697700
auto path = uri_to_path(params.text_document.uri);
698701
auto path_id = path_pool.intern(path);
699702

700-
co_await ensure_compiled(path_id, params.text_document.uri);
703+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
704+
co_return serde_raw{"null"};
701705

702706
worker::FoldingRangeParams wp;
703707
wp.path = path;
@@ -718,7 +722,8 @@ void MasterServer::register_handlers() {
718722
auto path = uri_to_path(params.text_document.uri);
719723
auto path_id = path_pool.intern(path);
720724

721-
co_await ensure_compiled(path_id, params.text_document.uri);
725+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
726+
co_return serde_raw{"null"};
722727

723728
worker::DocumentSymbolParams wp;
724729
wp.path = path;
@@ -739,7 +744,8 @@ void MasterServer::register_handlers() {
739744
auto path = uri_to_path(params.text_document.uri);
740745
auto path_id = path_pool.intern(path);
741746

742-
co_await ensure_compiled(path_id, params.text_document.uri);
747+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
748+
co_return serde_raw{"null"};
743749

744750
worker::DocumentLinkParams wp;
745751
wp.path = path;
@@ -760,7 +766,8 @@ void MasterServer::register_handlers() {
760766
auto path = uri_to_path(params.text_document.uri);
761767
auto path_id = path_pool.intern(path);
762768

763-
co_await ensure_compiled(path_id, params.text_document.uri);
769+
if(!co_await ensure_compiled(path_id, params.text_document.uri))
770+
co_return serde_raw{"null"};
764771

765772
worker::CodeActionParams wp;
766773
wp.path = path;
@@ -781,8 +788,9 @@ void MasterServer::register_handlers() {
781788
auto path = uri_to_path(params.text_document_position_params.text_document.uri);
782789
auto path_id = path_pool.intern(path);
783790

784-
co_await ensure_compiled(path_id,
785-
params.text_document_position_params.text_document.uri);
791+
if(!co_await ensure_compiled(path_id,
792+
params.text_document_position_params.text_document.uri))
793+
co_return serde_raw{"null"};
786794

787795
worker::GoToDefinitionParams wp;
788796
wp.path = path;

src/server/worker_pool.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,28 @@ template <typename Params>
9090
RequestResult<Params> WorkerPool::send_stateful(std::uint32_t path_id,
9191
const Params& params,
9292
et::ipc::request_options opts) {
93+
if(stateful_workers.empty()) {
94+
co_return et::outcome_error(et::ipc::Error{"No stateful workers available"});
95+
}
9396
if(!opts.timeout.has_value()) {
9497
opts.timeout = kWorkerRequestTimeout;
9598
}
9699
auto idx = assign_worker(path_id);
97-
return stateful_workers[idx].peer->send_request(params, opts);
100+
co_return co_await stateful_workers[idx].peer->send_request(params, opts);
98101
}
99102

100103
template <typename Params>
101104
RequestResult<Params> WorkerPool::send_stateless(const Params& params,
102105
et::ipc::request_options opts) {
106+
if(stateless_workers.empty()) {
107+
co_return et::outcome_error(et::ipc::Error{"No stateless workers available"});
108+
}
103109
if(!opts.timeout.has_value()) {
104110
opts.timeout = kWorkerRequestTimeout;
105111
}
106112
auto idx = next_stateless;
107113
next_stateless = (next_stateless + 1) % stateless_workers.size();
108-
return stateless_workers[idx].peer->send_request(params, opts);
114+
co_return co_await stateless_workers[idx].peer->send_request(params, opts);
109115
}
110116

111117
template <typename Params>

tests/unit/server/worker_test_helpers.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,13 @@ struct WorkerHandle {
112112
};
113113

114114
auto result = et::process::spawn(opts, loop);
115-
if(!result)
115+
if(!result) {
116+
#ifndef _WIN32
117+
if(stderr_fd >= 0)
118+
::close(stderr_fd);
119+
#endif
116120
return false;
121+
}
117122

118123
auto& spawn = *result;
119124
transport = std::make_unique<et::ipc::StreamTransport>(std::move(spawn.stdout_pipe),

0 commit comments

Comments
 (0)