codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug: [Session] Redis - write_close_session #7695

Closed gabriel-joy closed 1 year ago

gabriel-joy commented 1 year ago

PHP Version

8.2

CodeIgniter4 Version

4

CodeIgniter4 Installation Method

Manual (zip or tar.gz)

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

No response

What happened?

switching session filehandler to redis handler works as exepected as long as I'm not using session_write_close.

calling session_write_close in the same script works fine but in ajax calls I get an empty session.

after session_write_close, session->destroy() generate an exception as well

Steps to Reproduce

  1. switch to redis file handler set in public $sessionDriver = 'CodeIgniter\Session\Handlers\RedisHandler'; public string $sessionSavePath = 'tcp://localhost:6379?database=10'
  2. set session variable in main script call session_write_close()

  3. make an ajax call and check session variables.

Expected Output

to work like file handler

Anything else?

No response

kenjis commented 1 year ago

What is the exact error message? How did you make an ajax call?

It would be helpful if you could show a minimal working sample code to reproduce the error.

gabriel-joy commented 1 year ago

Not exactly my case but very close:

Just create 2 functions in your controller and call one after the other in browser (or uncomment the redirect line): With redis the session simply not working.

public function debug1()
    {

        $session = \Config\Services::session();
        $session->start();

        $session->set("test","hola");
        session_write_close();

        $session = \Config\Services::session();
        $session->start();
        print_r($_SESSION, false);
        session_write_close();

           //return redirect()->redirect( base_url('home/debug2'));
    }

    public function debug2()
    {

        $session = \Config\Services::session();
        $session->start();

        print_r($_SESSION, false);
        session_write_close();
    }
ping-yee commented 1 year ago

Out off the topic, how do I make sure the session store the data into Redis is working?

Because all operations of the session are performed normally after I set up the redisHandler in Config, but I find that there is no data stored in Redis.

gabriel-joy commented 1 year ago

https://stackoverflow.com/questions/19257573/how-do-i-know-if-sessions-are-successfully-being-managed-by-redis

kenjis commented 1 year ago

@gabriel-joy Try #7887

gabriel-joy commented 1 year ago

Yes, it's working as expected.

Do you know when this fix will be published in the official release? Thank you.

kenjis commented 1 year ago

@gabriel-joy Thank you for testing! The fix will be included in v4.4.2. It would be released in the next month.

gabriel-joy commented 1 year ago

@kenjis There is a performance issue with this fix. In my app I have a complex dialog with multiple ajax calls, with REDIS I have Session read length:1.0053s. With FILES I have Session read length:0.0058s. Otherwise, everything else works fine. Therefore, the fix is partial, something is still missing. Sorry :(

My guess is the session is locked and all calls are sequential, session_write_close() not closing the session.

kenjis commented 1 year ago

The performance issue is really because of the fix?

gabriel-joy commented 1 year ago

how could I know? I will open a new bug and hopefully someone will find out.

kenjis commented 1 year ago

how could I know?

Revert the change: https://github.com/codeigniter4/CodeIgniter4/pull/7887/files#diff-1a7b1fe40ddc58f0b240a268889a1952778b0631ce9db5f54cddaa359429dd60 and benchmark.

gabriel-joy commented 1 year ago

I updated the script for performance tests, it seems that performance is not affected. Bot requests are executed in parallel not waiting for each other. I assume for the moment my findings were related to network conditions (mobile connection) and not to REDIS handler.

Here is the updated test code:

public function debug1()
    {
        $benchmark = \Config\Services::timer();
        $benchmark->start('read session1');

        $session = \Config\Services::session();
        $session->start();

        $session->set("test","hola");
        session_write_close();

        sleep(5);

        $session = \Config\Services::session();
        $session->start();
        print_r($_SESSION, false);
        session_write_close();

        $benchmark->stop('read session1');

        $timers = $benchmark->getTimers();
        echo('Session read length:'.$timers['read session1']['duration'].'s');
    }

    public function debug2()
    {
        $benchmark = \Config\Services::timer();
        $benchmark->start('read session2');

        $session = \Config\Services::session();
        $session->start();

        print_r($_SESSION, false);
        session_write_close();

        $benchmark->stop('read session2');
        $timers = $benchmark->getTimers();
        echo('Session read length:'.$timers['read session2']['duration'].'s');
    }