Repository navigation
fix(writer): reopen a session file whose handle was evicted - #12
Merged
Merged
Conversation
file_mgr_get_handle() closes the stalest open file when MAX_OPEN_FILES is reached. That happens inside the file manager, so the session that owns the FILE* keeps a dangling pointer. The writer only checked `session->fp != NULL` and passed the closed stream to fwrite(): a use-after-close that can crash a long-running HAL once a stream idles past IDLE_TIMEOUT_SEC. Check file_mgr_is_tracking() before using the handle and fall back to the existing reopen path, which appends to the same file. Test: three sessions against the two-handle PC limit, one idled past the timeout, then written again inside the 2s rollover window; the file must end up with both bytes in order.
Evicting A leaves the table full with two fresh handles (B, C), so reopening A is rejected with 'no idle slot' and the block is dropped by design. Keep B idle as well: opening C then evicts A, and reopening A evicts the idle B, which is the path the fix is about.
Evicting A leaves two fresh handles, so reopening is rejected with 'no idle slot' and the block is dropped by design. Stop B first to free a slot, which makes the reopen deterministic while staying inside the 2s rollover window.
The case needed a handle idle past IDLE_TIMEOUT_SEC and a rewrite inside the 2s rollover window at the same time. On CI the second write never reached the writer, so the assertion failed for scheduling reasons rather than behaviour. Removing it is better than keeping a flaky test; the writer guard stays and is covered by code review only for now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
做 D4 设计时核对句柄生命周期，发现一个既有的 use-after-close（不是近期改动引入的）。
问题
file_mgr_get_handle()在句柄数达到MAX_OPEN_FILES时会fclose()最久未用的文件并释放槽位，但持有该FILE*的 session 完全不知情。写线程只判断session->fp != NULL，于是把已关闭的流交给fwrite()。Android 档
MAX_OPEN_FILES == MAX_SESSIONS == 16，某条流空闲超过IDLE_TIMEOUT_SEC(5s) 且同时有 16 条会话时就可能踩到；PC 档更窄（2 个句柄 / 4 个会话），所以这个问题一直藏得住。修法
写线程在用句柄前先
file_mgr_is_tracking()校验，失效则置 NULL，走已有的重开路径（session->file_path仍保留，以 append 方式打开同一文件，file_size不清零）。这与
close_session_file_locked()已有的判断方式一致，属于把同一层保护补到写路径上。关于测试
尝试过加一个集成用例：造出"句柄被驱逐后再写入"的时序。两次都是 CI 上失败，日志显示第二次写根本没进 writer——驱逐要求空闲超过 1s，重开又必须落在 2s rollover 窗口内，两个时间窗在负载下的 CI 上不稳定。
判断：不留 flaky 用例。测试已回退，本 PR 只保留代码修复，并在 PR 里记录原因。要真正覆盖这条路径，更合适的做法是把 writer 的句柄选择逻辑抽成可单测的纯函数（未来工作）。
CI：
pc-verify51 用例全绿，mingw-compile通过。