Fixed PHP warnings when session backend exits before returning.

[colinmollenhour]

Description

When a session handler exits during session_start() the PHP engine still tries to end the session during shutdown even though it was never successfully started. The Cm_RedisSession was updated with a setDieOnError method (requires 3.1+) to allow this automatic exit to be disabled and this patch for OpenMage disables it and then catches any thrown exceptions and handles the exit more gracefully by first calling session_abort().

Fixed Issues

PHP throws a few warnings when exiting un-gracefully during session_start():

• Warning: Unknown: Failed to write session data using user defined save handler. (session.save_pat...
• Warning: Unknown: Cannot call session save handler in a recursive manner in Unknown on line 0

Manual testing scenarios

Setup Redis sessions in app/etc/local/xml:

<config>
    ...
    <global>
        ...
        <redis_session>
            <host>redis</host>
            <port>6379</port>
            <max_concurrency>3</max_concurrency>
        </redis_session>
    </global>
</config>

Pass a number of concurrent requests to overwhelm the concurrency limit:

hey -c 10 -n 10 -H 'Content-Type: application/x-www-form-urlencoded' \
  -H 'Cookie: XDEBUG_SESSION=phpstorm; adminhtml=647625dfe137cf209889b524fb177e6b' \
  -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36' \
  'http://openmage-7f000001.nip.io/index.php/admin/index/index/key/955d7347e9813e7994ca43c8b054003a/'

Summary:

Status code distribution:
  [200] 3 responses
  [503] 7 responses

Observe that a 503 error is returned for the excess requests and check if any warnings are logged due to the session backend errors.

[fballiano]

@colinmollenhour do you think we could target v19 or there's something against it?

[elidrissidev]

Could this be possibly related to #3030?

[colinmollenhour]

do you think we could target v19 or there's something against it?

It is mostly BC, but will result in minor changes of behavior for error handling if exceptions are throw from the session handler. Namely, all throwable are caught and will cause immediate exit now instead of bubbling up. I don't know what those exceptions would be, they should be rare in my estimation and only affect errors, not normal requests. In the case of Cm_RedisSession it was already catching exceptions and exiting on it's own. Also, the latest Cm_RedisSession drops support for PHP <7.4, although so does OpenMage (it breaks the dev/openmage docker environment as that is still running PHP 7.3..).

So, I think this would fall under a MINOR update, not a MAJOR or PATCH:

functionality: if it doesn't fix a significant regression, it doesn't get a PATCH

This doesn't fix a regression, more of a nuisance.. So based on the new RFC (which hasn't quite passed yet) this would go to "main" and not get backported, in my opinion.

Could this be possibly related to #3030?

Yes, somewhat related. However, those errors don't occur under normal conditions and seem like a symptom of some other problem as in normal Magento code I don't see why there should be any output before the session has even been started yet. So somewhat related but not exactly.

[fballiano]

@colinmollenhour anyway branch 19.4.x is now on php7.4

The base branch was changed.