Skip to content

ext/session: Fix session_write_close_encode_failure.phpt on Solaris - #24401

Open
jorgsowa wants to merge 1 commit into
php:PHP-8.6from
jorgsowa:fix-session-flock-test-solaris
Open

jorgsowa wants to merge 1 commit into
php:PHP-8.6from
jorgsowa:fix-session-flock-test-solaris

Conversation

@jorgsowa

Copy link
Copy Markdown
Contributor

The test checks the session lock with flock($fp, LOCK_EX | LOCK_NB) on a file opened with fopen($file, 'r'). On platforms without flock(), such as Solaris, PHP falls back to fcntl() locks. fcntl() refuses an exclusive lock on a read-only file, so the check returns false even though the lock was released. Its flock() goes through the fcntl() wrapper in ext/standard/flock_compat.c, which is why the test fails.

Should fix Solaris issue in nigthly: https://github.com/php/php-src/actions/runs/38990675359/job/117272412935

var_dump(session_write_close());

$fp = fopen($file, 'r');
$fp = fopen($file, 'r+');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but then this test will always "succeed" on solaris, flock is emulated on solaris and is in the same process as session_write_close. Probably best to skip solaris instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's not the point of the test. It's testing ext/session, not the fopen or flock. Those handlers here are for manipulating session file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you make rerun the CI for solaris ? it was skipped

@jorgsowa
jorgsowa force-pushed the fix-session-flock-test-solaris branch from 069ed34 to 9882ddb Compare October 18, 2026 18:32
Exclusive flock() requires a writable descriptor on Solaris.
@jorgsowa
jorgsowa force-pushed the fix-session-flock-test-solaris branch from 9882ddb to 298d857 Compare October 18, 2026 19:55
@jorgsowa

Copy link
Copy Markdown
Contributor Author

The fixed test doesn't fail anymore. There are still two failures related to proc_open().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants