Skip to content

Commit 5b534ce

Browse files
fix(mcp): address QA round 2 findings
- Add explicit fallback path when fcntl(F_GETFL) fails: skip the FILE* peek and fall through directly to blocking poll so idle eviction still fires on timeout (Finding 1) - Strengthen C unit test: verify id:1 (initialize) and id:2 (tools/list) response IDs are both present, not just a substring match on "tools" (Finding 2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5ca5f32 commit 5b534ce

2 files changed

Lines changed: 32 additions & 17 deletions

File tree

src/mcp/mcp.c

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2425,20 +2425,10 @@ int cbm_mcp_server_run(cbm_mcp_server_t *srv, FILE *in, FILE *out) {
24252425
* would block indefinitely, preventing the Phase 3 idle eviction
24262426
* timeout from ever firing. */
24272427
int saved_flags = fcntl(fd, F_GETFL);
2428-
if (saved_flags >= 0) {
2429-
(void)fcntl(fd, F_SETFL, saved_flags | O_NONBLOCK);
2430-
}
2431-
int c = fgetc(in);
2432-
if (saved_flags >= 0) {
2433-
(void)fcntl(fd, F_SETFL, saved_flags); /* restore blocking */
2434-
}
2435-
if (c == EOF) {
2436-
if (feof(in)) {
2437-
break; /* true EOF */
2438-
}
2439-
/* No buffered data (EAGAIN from non-blocking read) — clear the
2440-
* ferror indicator set by EAGAIN, then do the blocking poll. */
2441-
clearerr(in);
2428+
if (saved_flags < 0) {
2429+
/* fcntl failed (should not happen on a valid fd) — skip the
2430+
* FILE* peek and fall straight through to the blocking poll so
2431+
* idle eviction still fires on timeout. */
24422432
pr = poll(&pfd, 1, STORE_IDLE_TIMEOUT_S * 1000);
24432433
if (pr < 0) {
24442434
break;
@@ -2448,8 +2438,28 @@ int cbm_mcp_server_run(cbm_mcp_server_t *srv, FILE *in, FILE *out) {
24482438
continue;
24492439
}
24502440
} else {
2451-
/* Buffered data found — push back and fall through to getline */
2452-
(void)ungetc(c, in);
2441+
(void)fcntl(fd, F_SETFL, saved_flags | O_NONBLOCK);
2442+
int c = fgetc(in);
2443+
(void)fcntl(fd, F_SETFL, saved_flags); /* restore blocking */
2444+
if (c == EOF) {
2445+
if (feof(in)) {
2446+
break; /* true EOF */
2447+
}
2448+
/* No buffered data (EAGAIN from non-blocking read) — clear
2449+
* the ferror indicator set by EAGAIN, then blocking poll. */
2450+
clearerr(in);
2451+
pr = poll(&pfd, 1, STORE_IDLE_TIMEOUT_S * 1000);
2452+
if (pr < 0) {
2453+
break;
2454+
}
2455+
if (pr == 0) {
2456+
cbm_mcp_server_evict_idle(srv, STORE_IDLE_TIMEOUT_S);
2457+
continue;
2458+
}
2459+
} else {
2460+
/* Buffered data found — push back and fall through to getline */
2461+
(void)ungetc(c, in);
2462+
}
24532463
}
24542464
}
24552465
#endif

tests/test_mcp.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1272,11 +1272,16 @@ TEST(mcp_server_run_rapid_messages) {
12721272

12731273
ASSERT_EQ(rc, 0);
12741274

1275-
/* Verify that tools/list response is present in output */
1275+
/* Verify both responses are present:
1276+
* id:1 — initialize response
1277+
* id:2 — tools/list response (notifications/initialized produces none)
1278+
* and that the tools list payload is included. */
12761279
rewind(out_fp);
12771280
char buf[4096] = {0};
12781281
size_t nread = fread(buf, 1, sizeof(buf) - 1, out_fp);
12791282
ASSERT_TRUE(nread > 0);
1283+
ASSERT_NOT_NULL(strstr(buf, "\"id\":1"));
1284+
ASSERT_NOT_NULL(strstr(buf, "\"id\":2"));
12801285
ASSERT_NOT_NULL(strstr(buf, "tools"));
12811286

12821287
cbm_mcp_server_free(srv);

0 commit comments

Comments
 (0)