[php-src] PR #25002: ext/session: Do not GC a session file another request still holds locked
| From: | AsahiSoftWareEngineer | Date: | Wed, 28 Oct 2026 14:55:34 +0000 |
| Subject: | [php-src] PR #25002: ext/session: Do not GC a session file another request still holds locked | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-40025@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/25002
Author: AsahiSoftWareEngineer
### Description
ps_files_cleanup_dir() (the files session save handler's garbage
collector) deletes any sess_* file whose mtime is older than
gc_maxlifetime without checking whether another request currently has that file open
and locked via flock(LOCK_EX) -- a lock ps_files_open() holds for the
entire lifetime of a session.
On POSIX, unlink() on an open file succeeds silently: the directory entry disappears,
but the holder's already-open file descriptor keeps working against the now-orphaned inode.
session_write_close() therefore reports success while the session data is permanently
lost, and neither side ever sees an error.
See #25001 for the full writeup, reproduction steps, and reachability analysis (every ordinary
session_start() call runs this sweep internally with
gc_probability/gc_divisor odds, so it's reachable without any special
access).
### Changes
- ext/session/mod_files.c: before deleting an expired candidate, probe it with a
non-blocking flock(LOCK_EX | LOCK_NB); skip it if another process still holds it open
instead of unlinking it. Only increment the deletion counter once unlink() actually
succeeds (its return value was previously discarded unconditionally).
- Added ext/session/tests/session_gc_active_session_race.phpt: creates a session file,
backdates its mtime past gc_maxlifetime, holds it open+locked via a second
fopen()/flock() handle, runs session_gc() from an unrelated
session, and asserts the file -- and its data -- survive.
- NEWS entry.
### Checklist
- [x] Regression test included
- [x] NEWS entry added
- [ ] No existing test coverage needed updating (new behavior is additive: previously-untested
concurrent-access case)
Closes #25001