swooletw / laravel-swoole

High performance HTTP server based on Swoole. Speed up your Laravel or Lumen applications.
MIT License
4.03k stars 388 forks source link

facades are not safe to use inside coroutine for sure in this package. #549

Closed videni closed 9 months ago

videni commented 9 months ago

I don't find any document about this subject, so I read some critical files:

src/Server/Manager.php
src/Server/Sandbox.php
src/Coroutine/Context.php

what I got

Every request will create a sandbox which clones the base container and restores it back to the orginal after request finished. as I know, the onRequest event of \Swoole\Http\Server runs inside a coroutine. what concerns me is that when a sandbox is enabled, the setInstance method below will set the shared static variable instance of container(src/Illuminate/Container/Container.php ) to the one created by the sandbox. does this mean it also change another request?

src/Server/Sandbox.php


 public function enable()
    {
        //...
        $this->setInstance($app = $this->getApplication());
        $this->resetApp($app);
    }

    public function setInstance(Container $app)
    {
        $app->instance('app', $app);
        $app->instance(Container::class, $app);

           //...
        Container::setInstance($app);
        Context::setApp($app);
        Facade::clearResolvedInstances();
        Facade::setFacadeApplication($app);
    }

src/Illuminate/Container/Container.php

class Container implements ArrayAccess, ContainerContract
{
            //...

    protected static $instance;
           //...

}

Related issues

videni commented 9 months ago

I did an experiment, it proves my guess, facade is not safe, this library don't do any thing to handle this case. you must be very carefull . here is a picture of my experiment.

log instance resets for every request. you can see the first log instance of a request can be reset by another one.

ENlOtTCU3h

here is my code.

class IndexController extends BaseController
{
    public function first()
    {
        Facade::log('## 1. '.' First log instance'.'-cid-'.\Swoole\Coroutine::getCid());

         // Will create one from the container of current sandbox
        Log::info('Hello World!');

       // Will use the cached instance of static resolvedInstance variable of  class,  Illuminate\Support\Facades\Facade
        Log::info('Hello World!');

        Facade::log('I will sleep 3 seconds');

        \Swoole\Coroutine::sleep(3);

        Facade::log('## 1.1. '.'I wake up'.'-cid-'.\Swoole\Coroutine::getCid());

        Log::info('Hello World!');

        Log::info('Hello World!');

        return 'Hello World!';
    }

    public function second()
    {
        Facade::log('## 2. '.' Second log instance'.'-cid-'.\Swoole\Coroutine::getCid());

        Log::info('Hello World!');

        Log::info('Hello World!');

        Facade::log('I will sleep 6 seconds');

        \Swoole\Coroutine::sleep(6);

        Facade::log('## 2.1. '.'I wake up'.'-cid-'.\Swoole\Coroutine::getCid());

        Log::info('Hello World!');

        Log::info('Hello World!');

        return 'Hello World!';
    }
}