0 max_execution_time ini value

[arisro]

It seems that zend_ini_long doesn't work anymore with ZEND_STRS
and that the extension should use ZEND_STRL, back again.

We inspected and stumbled upon this issue when some session had their lock stuck. It seems that the extension was unable to retrieve the INI value for max_execution_time.
This was at some point changed to ZEND_STRS - but it seems that now it has to use ZEND_STRL.

Anyone can probably easily test this and confirm that it is indeed an issue and that this fixes it.

[sodabrew]

Is this the same "session lock" issue that #269 is also reporting? @arisro what version of PHP are you using?

[arisro]

@sodabrew We are currently using PHP 7.0.6.

I debugged the extension and indeed it doesn't properly read the value of max_execution_time and sets the lock with 0 TTL. With the change in this PR it successfully retrieves the value for max_execution_time and uses that.

I think the best approach would be to also set memcached.sess_lock_expire to a non-0 value by default - to avoid setting 0 TTL locks when even max_exection_time is really 0. The user should intentionally set the value on 0 if he REALLY wants 0 TTL locks.

Our problem was that very rarely the requests were failing (like apache2 processes dying for example) and there was no session_write_close() being done - and the lock keys were stuck forever.

[sodabrew]

ZEND_STRS -- Pass string pointer and buffer size in one operation

ZEND_STRL -- Pass string pointer and string length in one operation

long zend_ini_long ( char* name, uint name_length, int orig )

This would be a revert of https://bugs.php.net/bug.php?id=59641 where the same change was made in the other direction.

[arisro]

@sodabrew I tried to manually pass the string and the length arguments and then I saw that fix and I tired to revert it and it worked.

(I have no real experience with PHP extension development - so I'm not of that much help.)

[sodabrew]

This smells more of a bug in PHP 7. Passing a nul-terminated string to zend_ini_long should work.

Oh sweet. It's internally inconsistent:
https://github.com/php/php-src/search?utf8=%E2%9C%93&q=zend_ini_long

See how these macros are defined? They're explicitly subtracting the length of the final nul byte:

ZEND_API zend_long zend_ini_long(char *name, uint name_length, int orig);
… 
139 #define INI_INT(name) zend_ini_long((name), sizeof(name)-1, 0)
140 #define INI_FLT(name) zend_ini_double((name), sizeof(name)-1, 0)

And yet an in-tree extension (SOAP) does this (sizeof includes the nul byte in the length):

        if (zend_ini_long("zlib.output_compression", sizeof("zlib.output_compression"), 0))

[sodabrew]

Given how these are defined,

#define ZEND_STRL(str)      (str), (sizeof(str)-1)
#define ZEND_STRS(str)      (str), (sizeof(str))

And how the code clearly is saying that it wants the length of the string without the final nul byte, ZEND_STRL should have always been the correct usage. I cannot understand why it was changed to ZEND_STRS in the past.

[sodabrew]

The original ticket https://bugs.php.net/bug.php?id=59641 was resolved by @andreiz maybe he recalls the issue?

[arisro]

Yup, that's what it looks like - it should've been ZEND_STRL.

Maybe some implementation changed in zend_hash. Looks like zlib.output_compression could be affected too.

[sodabrew]

@arisro Could I ask you to rebase this to current master, as the php7 branch has merged?

[arisro]

@sodabrew Done! :)