Hacktoberfest 2026:维护者为十月标记出来的 issue,仍然开放、适合新手。 浏览 Hacktoberfest issue

[Server] Concurrent requests corrupt the session file in FileSessionStore, crashing Session::readData() with JsonException

未关闭
#498 2 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
4/5
预计耗时
3-5 天
新手友好度
68/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
php
领域
backend

调研方向

从 src/Server/Session/Session.php 以及 FileSessionStore::write() 和 read() 路径开始,然后跟踪从 Protocol.php 经过 BaseTransport::getOutgoingMessages() 的第二次 session 读取。重现 issue 中描述的空文件情况和并行的 tools/list 请求。完成的标准是并发写入不会发布空文件或不完整文件,并且空的 store 数据会被视为空 session 处理,而不会出现未捕获的 JsonException。

由索引模型根据 Issue 内容生成。

描述

bug P2 Server

Describe the bug

Under PHP-FPM, two concurrent requests carrying the same Mcp-Session-Id can leave the session file empty (0 bytes). The next read of that session throws an uncaught JsonException out of Session::readData(), and the server answers -32603 for a tool call that had already completed successfully — the tool ran, its response was queued in the session, and then the response could not be retrieved.

Two independent defects combine here; each is a necessary condition.

  1. FileSessionStore::write() uses a fixed temporary filename. $tmp = $path.'.tmp'; is identical for every concurrent writer of the same session, and file_put_contents() opens the stream in w mode — so truncation happens before LOCK_EX is acquired. One process can therefore rename() a temp file that another process has just truncated to 0 bytes, publishing an empty session file. The // Atomic move comment applies to rename() alone; the operation as a whole is not atomic because the temp name is shared. The copy() fallback has the same problem (it truncates the destination and then streams into it), and read() takes no LOCK_SH.

  2. Session::readData() does not guard against an empty string. Only false is handled, so '' reaches json_decode(..., JSON_THROW_ON_ERROR):

$rawData = $this->store->read($this->id);
if (false === $rawData) { return $this->data = []; }        // only false is handled
$decoded = json_decode($rawData, true, flags: \JSON_THROW_ON_ERROR);   // throws on ''
if (!\is_array($decoded)) { return $this->data = []; }      // unreachable for ''

FileSessionStore::read() returns '' for a zero-byte file — it returns false only when the file is missing, expired, or unreadable. This half is store-agnostic: any store that can return an empty string triggers it.

To Reproduce

Steps to reproduce the behavior:

  1. Run a server on PHP-FPM with StreamableHttpTransport and FileSessionStore (handshake era).
  2. POST an initialize request without a session id; take Mcp-Session-Id from the response headers.
  3. POST notifications/initialized with that header.
  4. Deterministic variant — truncate the session file by hand: : > <session-dir>/<session-id> (or write invalid JSON: printf '{"a":' > <session-dir>/<session-id>).
  5. POST tools/list with the same Mcp-Session-Id. The request fails with -32603 and the JsonException below.
  6. Concurrency variant (shows the actual cause rather than the symptom) — instead of step 4, fire ~40 parallel tools/list POSTs sharing one Mcp-Session-Id, e.g. seq 40 | xargs -P 8 -I{} curl -s -o /dev/null -w '%{http_code}\n' -X POST ..., and repeat a few rounds. Occasional requests fail with -32603.

Expected behavior

  1. Concurrent writes to the same session never publish a partial or empty session file.
  2. An empty or undecodable payload from the store degrades to an empty session instead of a fatal error. The existing !\is_array($decoded) branch already shows this is the intended behaviour — it is simply unreachable while JSON_THROW_ON_ERROR fires first.

Logs

JsonException: Syntax error in src/Server/Session/Session.php:168
#0 Session.php(168): json_decode('', true, 512, 4194304)
#1 Session.php(53):  Mcp\Server\Session\Session->readData()
#2 Protocol.php(517): Mcp\Server\Session\Session->get(Array, Array)
#3 BaseTransport.php(81): Mcp\Server\Protocol->consumeOutgoingMessages(Symfony\Component\Uid\UuidV4)
#4 StreamableHttpTransport.php(218): Mcp\Server\Transport\BaseTransport->getOutgoingMessages(...)
#5 StreamableHttpTransport.php(202): ...->createJsonResponse()
#6 StreamableHttpTransport.php(437): ...->handlePostRequest('{"jsonrpc":"2.0...')
#7 Server.php(67): Mcp\Server\Transport\StreamableHttpTransport->listen()

Additional context

Why the exception escapes every catch block. The crash happens in Protocol::consumeOutgoingMessages(), which builds a fresh Session instance:

// Protocol.php:516
$session = $this->sessionManager->createWithId($sessionId);
$queue = $session->get(self::SESSION_OUTGOING_QUEUE, []);   // second, independent disk read

A single POST therefore reads the session file twice, through two objects that share no memory — and the second read happens after the request's own save() at Protocol.php:223. That second read is reached from BaseTransport::getOutgoingMessages() while the response is being built, which is outside Protocol::processInput()'s try/catch (that one wraps only doProcessInput()); Server::run() has just try/finally. The exception propagates out of Server::run() into the application's own error handler.

Silent lost writes. After the losing process's rename() fails (its temp file is gone) and copy() fails too, write() returns false — but Session::save()'s return value is discarded at Protocol.php:223, :519, :551 and :644, so the failed write is invisible.

Where a fix would belong. Both halves look small and independent — the shared temp filename in FileSessionStore::write(), and the missing '' case in Session::readData().

One more observation, separate from the bug. Since #479, gc() skips anything whose filename is not a valid UUID, so orphaned *.tmp files are never cleaned up. Harmless at one temp file per session, but worth knowing if the temp name ever becomes unique.

Related but distinct issues. #275 (lost update on concurrent read-modify-write) and #467 (concurrent POSTs returning a JSON array). A per-session lock as proposed in #275 would incidentally prevent this crash, but would not close item 2.

Environment. mcp/sdk v0.8.1, PHP 8.5, PHP-FPM, StreamableHttpTransport with withoutModernEra(), FileSessionStore on local disk (Linux), TTL 14400 s. The same code is present on main, so this is not fixed by upgrading.

主要语言
PHP
星标
1.6k
派生
173
平均合并
19 小时 19 分钟
30 天内合并 PR
8

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue,再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库,在一个分支上完成修改。
  4. 提交 Pull Request,并在描述里引用这个 Issue 编号。

modelcontextprotocol/php-sdk 的其他 Issue

查看 modelcontextprotocol/php-sdk 的全部 Issue

相似的 Issue

更多 PHP Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。