fix: KernelFactory — Singleton Kernel Without State Reset (closes #22)

[s2x]

Summary

• Adds services_resetter call after kernel->terminate() to reset services between requests
• Prevents memory leaks and stale state in long-running processes (EntityManager identity map, request-scoped services)
• Gracefully handles missing services_resetter (test mocks, minimal configs)

Changes

• src/Middleware/SymfonyController.php - Added resetServices() method called after terminate
• tests/SymfonyControllerTest.php - Added tests for service reset functionality

Testing

• All 265 tests pass (11 skipped for Symfony version compatibility)
• Tests verify:
  • Services reset is called when services_resetter is available
  • No errors when services_resetter is not available (backward compatible)

[s2x]

[Suggestion] Double container lookup (has() + get()) on the hot path — resetServices() performs two separate service-locator lookups per request:

$container->has('services_resetter')  // first lookup
$container->get('services_resetter')  // second lookup

Since the presence of services_resetter is a static property of the application's service configuration (it never changes between requests), this double-lookup repeats unnecessary work on every single request.

In a high-throughput Workerman server, caching the services_resetter reference after the first lookup would eliminate this per-request overhead. For example, cache the resolved ResetInterface (or null if unavailable) in a private property after the first call:

private ?ResetInterface $servicesResetter = null;
private bool $servicesResetterInitialized = false;

private function resetServices(): void
{
    if (!$this->servicesResetterInitialized) {
        try {
            $container = $this->kernel->getContainer();
            if ($container->has('services_resetter')) {
                $resetter = $container->get('services_resetter');
                if ($resetter instanceof ResetInterface) {
                    $this->servicesResetter = $resetter;
                }
            }
        } catch (\Throwable) {
            // log or handle
        }
        $this->servicesResetterInitialized = true;
    }
    $this->servicesResetter?->reset();
}

— coder-model via Qwen Code /review

[s2x]

[Suggestion] catch (\Throwable) silently swallows all exceptions, including Error subclasses — with an empty body, there is zero observability into reset failures.

This is problematic because:

1. Contradicts the PR's goal. The PR aims to prevent memory leaks in long-running processes. But if the container is in an inconsistent state or the resetter throws, the failure is invisible — services never get reset, and memory leaks accumulate with no trace.
2. No debugging trail. If getContainer() returns something unexpected, or has()/get() throws for a reason other than service absence, the failure disappears completely.
3. Asymmetric error handling. The HttpRequestHandler wraps terminateIfNeeded() in try-catch with error_log() — but because resetServices() absorbs its own exceptions, reset failures never reach that logging.

Recommended fix — at minimum, log the exception:

} catch (\Throwable $e) {
    error_log(sprintf(
        '[workerman-bundle] Failed to reset services: %s in %s:%d',
        $e->getMessage(),
        $e->getFile(),
        $e->getLine(),
    ));
}

Alternative — narrow the catch to ContainerExceptionInterface and re-throw other exceptions, letting the caller's existing error handling take over. This ensures structural errors (e.g., container API misuse) are not silently ignored.

— coder-model via Qwen Code /review

[s2x]

[Suggestion] TestTerminableKernel::$servicesResetCalled is dead code — the property is declared but never written to or asserted.

TestTerminableKernel::getContainer() throws RuntimeException('Not implemented'), which resetServices() silently catches. So $servicesResetCalled stays false forever. The property appears to have been copied from TestKernelWithServicesResetter, but it serves no purpose here.

Fix: Remove public bool $servicesResetCalled = false; from TestTerminableKernel to avoid misleading readers.

— coder-model via Qwen Code /review

[s2x]

[Suggestion] testTerminateIfNeededDoesNotCallServicesResetterWhenNotAvailable doesn't actually test the 'service unavailable' path.

The test uses TestTerminableKernel, whose getContainer() throws RuntimeException('Not implemented'). The test passes because resetServices() swallows the exception — but the assertion only verifies $kernel->terminateCalled. The test never reaches the has('services_resetter') check; it only verifies that exception-swallowing works.

This gives false confidence that the 'service not available' branch is correct. If the exception-handling behavior were ever changed (e.g., per the logging suggestion above), this test would fail unexpectedly.

Fix: Use a kernel mock whose getContainer() returns a container where has('services_resetter') returns false. This exercises the actual availability check rather than an exception path. Alternatively, rename the test to reflect what it actually verifies.

— coder-model via Qwen Code /review