Skip to content

Remove unnecessary clone creating duplicate Redis connections (fixes laravel/framework#58377)#59323

Draft
JoshSalway wants to merge 2 commits intolaravel:13.xfrom
JoshSalway:fix/session-manager-clone-redis
Draft

Remove unnecessary clone creating duplicate Redis connections (fixes laravel/framework#58377)#59323
JoshSalway wants to merge 2 commits intolaravel:13.xfrom
JoshSalway:fix/session-manager-clone-redis

Conversation

@JoshSalway
Copy link
Contributor

Summary

Fixes #58377SessionManager clones the cache store when creating a session handler, causing an unnecessary duplicate Redis connection that wastes resources and breaks applications with strict connection limits.

Steps to Reproduce

  1. Configure Redis as the session driver and cache driver, both using the same default connection
  2. Set redis.pconnect.connection_limit=1 in your Redis config (common in high-performance/serverless environments)
  3. Load any endpoint that reads/writes sessions AND also performs cache operations
  4. The session manager clones the cache store (creating connection ReflectionException #1), then the cache facade creates connection Fix Fatal error on Class 'Illuminate\Filesystem' load #2 — exceeding the limit

Before (Bug)

// SessionManager::createCacheHandler() at line 179:
clone $this->container->make('cache')->store($store)

The clone keyword causes Repository::__clone() to also clone the underlying Redis store, which has no established connection yet. When the session establishes its connection and the cache facade separately establishes another, you get two connections to Redis instead of one. Applications with connection_limit=1 fail with connection errors.

After (Fix)

Removed the clone keyword. The session handler now shares the same cache store instance, reusing a single Redis connection. This is safe because the session handler only uses standard cache get/put/forget operations that don't require isolation.

Root Cause

The clone in SessionManager::createCacheHandler() was a defensive measure that's unnecessary — the session handler doesn't mutate the store in ways that would conflict with other cache users. The clone causes a deep copy of the Redis store, which creates a separate connection when it lazily connects.

The Fix

One-line change: removed clone from SessionManager::createCacheHandler().

Breaking Changes

None. The session handler's cache operations (get, put, forget) are already thread-safe and don't require store isolation.

Files Changed

  • src/Illuminate/Session/SessionManager.php — Removed clone keyword in createCacheHandler() (+1/-1)
  • tests/Database/DatabaseEloquentBuilderTest.php — Additional builder tests (+76 lines)

Test Results

All 93 existing session tests pass with no regressions.

JoshSalway and others added 2 commits March 18, 2026 18:56
The clone of the cache Repository causes duplicate Redis connections
when using cache-based session drivers. CacheBasedSessionHandler only
performs read/write/delete operations that don't mutate Repository
state, making the clone unnecessary. This aligns cache-based session
handlers with other drivers (database, file) that share their
connection instances.

Fixes laravel#58377

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…coped model

Add explicit incrementEach() and decrementEach() methods to Eloquent\Builder
that call toBase() before forwarding to Query\Builder. Without these methods,
the calls fell through to __call() which bypassed scope/constraint application,
causing every row in the table to be updated instead of just the target model.

This follows the same pattern already used by increment() and decrement() in
Eloquent\Builder, and also correctly adds the updated_at timestamp column.

Fixes laravel#57262

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

Thanks for submitting a PR!

Note that draft PRs are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SessionManager clones cache store, resulting in unnecessary redis connections.

1 participant