Skip to content

Commit 3a8b3ab

Browse files
Alexey PortnovAlexey Portnov
authored andcommitted
fix(workers): drop $redis property overrides that break PHP 8.2+ covariance
Regression hotfix for #1022. Commit bc4875c introduced `protected mixed $redis = null;` on WorkerBase as part of the Redis connection hardening work (code review HIGH #2: Injectable::__get can resurrect a socket inside destructors unless the property is declared). That was correct in isolation, but three child classes already had their own legacy `protected $redis;` declaration without a type, and PHP 8.2+ property-type covariance requires the type to match exactly through the whole inheritance chain. Starting from the next cron tick after bc4875c hit a PHP 8.2+ server, loading any of the following classes produced a fatal: Type of MikoPBX\Core\Workers\Cron\WorkerSafeScriptsCore::$redis must be mixed (as in class MikoPBX\Core\Workers\WorkerBase) in /offload/rootfs/usr/www/src/Core/Workers/Cron/WorkerSafeScriptsCore.php:68 Same fatal on WorkerExtensionStatusMonitor and WorkerProviderStatusMonitor. Concrete impact observed on serber@boffart.miko.ru: - WorkerSafeScriptsCore is the watchdog that cron fires every minute. Every cron tick produced four stacked Whoops fatals (one per parallel instance) in /storage/usbdisk1/mikopbx/log/system/messages and exited before doing any work. New watchdog instances could not start. - The existing pre-regression watchdog process (the one that had been loaded into memory BEFORE bc4875c was deployed) kept running on stale bytecode, so supervision continued to "work" until it would eventually die — at which point the whole worker pool would lose its babysitter silently. - WorkerExtensionStatusMonitor and WorkerProviderStatusMonitor hit the same fatal on every status check; they are used for page-activity tracking in the admin cabinet. - Module safe.php cron jobs that share the core worker bootstrap were affected by the same covariance check on every minute tick. Fix is the minimal diff: delete the untyped `protected $redis;` override in the three child classes. They now inherit the typed `protected mixed $redis = null;` from WorkerBase, which is semantically identical to their old behaviour (mixed accepts anything including \Redis and Phalcon cache wrappers) and satisfies the PHP 8.2+ covariance rule. A short comment is left at each site so nobody reintroduces the override by accident. Verified on serber@boffart.miko.ru (MikoPBX 2026.1.233-dev, PHP 8.4.16): 1. Before: `class_exists('MikoPBX\Core\Workers\Cron\WorkerSafeScriptsCore')` throws `Whoops\Exception\ErrorException: ... must be mixed`. 2. After deploy: the same class_exists returns true, the direct `new WorkerSafeScriptsCore()` in a dry-run script constructs and clean-exits in 10 ms. 3. Pre-regression watchdog PID (30556) killed explicitly; cron respawned a fresh PID 5211 which picked up the hotfixed bytecode and is actively running the supervision loop (confirmed through `Checking worker pool: WorkerApiCommands - 3/3 instances running` and successful restarts of stale workers including WorkerRemoveOldRecords / WorkerS3CacheCleaner / WorkerNotifyAdministrator / ModuleCTIClient\WorkerSafeScript). 4. Zero new `must be mixed` fatals in syslog for the full observation window after the kill+respawn. Swept the rest of src/Core/Workers for any other untyped `$redis` overrides — none remain. `WorkerPoolManager::$redis` is declared `private Redis $redis` and does not extend WorkerBase, so it is unaffected.
1 parent 936d845 commit 3a8b3ab

3 files changed

Lines changed: 7 additions & 7 deletions

File tree

src/Core/Workers/Cron/WorkerSafeScriptsCore.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,9 @@ class WorkerSafeScriptsCore extends WorkerBase
111111
*/
112112
private array $pingFailureCounts = [];
113113

114-
/**
115-
* Redis connection instance
116-
* @var \Redis
117-
*/
118-
protected $redis;
114+
// Redis handle inherited from WorkerBase::$redis (protected mixed, default null).
115+
// A local override here would break PHP 8.2+ property type covariance —
116+
// see hotfix for issue #1022 WorkerSafeScriptsCore::$redis regression.
119117

120118
/**
121119
* Dedicated AMI connection for worker ping/pong checks.

src/Core/Workers/WorkerExtensionStatusMonitor.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ class WorkerExtensionStatusMonitor extends WorkerRedisBase
6161
];
6262

6363
private ?UserPageTrackerLib $pageTracker = null;
64-
protected $redis;
64+
// $redis inherited from WorkerBase (protected mixed $redis = null);
65+
// a local untyped override would break PHP 8.2+ property-type covariance.
6566
private int $lastCheckTime = 0;
6667

6768
/**

src/Core/Workers/WorkerProviderStatusMonitor.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ class WorkerProviderStatusMonitor extends WorkerRedisBase
6060
];
6161

6262
private ?UserPageTrackerLib $pageTracker = null;
63-
protected $redis;
63+
// $redis inherited from WorkerBase (protected mixed $redis = null);
64+
// a local untyped override would break PHP 8.2+ property-type covariance.
6465
private int $lastCheckTime = 0;
6566

6667
/**

0 commit comments

Comments
 (0)