Skip to content

fix: use dynamic lockMaxRetries limit in RedisHandler#10295

Open
gr8man wants to merge 9 commits into
codeigniter4:developfrom
gr8man:fix-redis-handler-lock-retries
Open

fix: use dynamic lockMaxRetries limit in RedisHandler#10295
gr8man wants to merge 9 commits into
codeigniter4:developfrom
gr8man:fix-redis-handler-lock-retries

Conversation

@gr8man

@gr8man gr8man commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

In RedisHandler::lockSession(), there was a bug where a hardcoded value of 300 was used to check if the maximum lock acquisition attempts were reached:

if ($attempt === 300) {

This bypassed the actual $this->lockMaxRetries property configured by the user. As a result, if a developer configured a smaller lockAttempts limit (e.g., 3), the check would fail to trigger because 3 !== 300, causing the method to incorrectly return true (success) despite not acquiring the lock. Conversely, it would hardcode the failure point to exactly 300 attempts regardless of higher configurations.

This PR replaces the hardcoded 300 with $this->lockMaxRetries in the condition and the associated error log message, ensuring the configured limits are appropriately enforced. A test has also been added to verify that the failure condition is triggered correctly after the maximum configured attempts.

Checklist:

  • Secure coding practices
  • Tests have been added to cover this change
  • All tests pass

@michalsn michalsn left a comment

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.

Good catch. Some small changes are required, as well as adding a changelog entry here: https://github.com/codeigniter4/CodeIgniter4/blob/develop/user_guide_src/source/changelogs/v4.7.4.rst

Comment thread system/Session/Handlers/RedisHandler.php Outdated
@michalsn michalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 10, 2026

@michalsn michalsn left a comment

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.

Please fix the changelog entry.

Comment thread user_guide_src/source/changelogs/v4.7.4.rst Outdated
Comment on lines +36 to 38
- **Session:** Fixed a bug in ``RedisHandler`` where the configured ``$lockRetryInterval`` and ``$lockMaxRetries`` values were not respected when acquiring session locks.
- **Model:** Fixed a bug in ``Model::objectToRawArray()`` where the ``$recursive`` parameter was ignored.

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.

We need alphabetical order.

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

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants