Re: Comments on non-unique naming convention for closures
Dmitry,
The underlying issue here is the assumption that build_runtime_defined_function_key() in zend_compile.c generates unique keys. I've realised that this impacts not only closures but also late-bound functions and classes. I've now produced PHPTs which demonstrates the failure in all three cases. Whilst hashing a closure source text has an acceptable compile-time overhead, the case for classes (with their larger source lengths) is more of a concern.
I am now proposing an alternative hash based on:
A. The class / function name (as per current)
B. The reference source file (as per current)
C. The address of the buffer (as per current)
D. The PID and microtime at the request start.
E. A running count of the function/classed compiled during the request.
(A-C) were assumed unique but, as #64291 and #65915 show, are not.
(D) should be sufficient to prevent race conditions across different OPcached threads / processes, but this has the advantage that it only needs to be computed once per request.
(E) guarantees uniqueness within thread.
The only weakness of D and E alone is a possible race on ZTS builds where two clashing threads start at the same microsecond, which (C) will split. If this is considered too unwieldy then we could always drop B as C-E guarantee uniqueness, and B is never exposed to the programmer anyway (Xdebug and the error logger use the appropriate fields in the zend_function / zend_class record and not its key in the corresponding EG table.
This also has the advantage that the main change can now be localised to build_runtime_defined_function_key().
//Terry
The following bugs relate to this discussion:
#64291 Indeterminate GC of evaled lambda function resources
#65915 Inconsistent results with require return value
I don't want to discuss these or the specific fixes here, since I can work up a fix and discuss them in the bugrep. However,
my one-sentence Q is "should we replace the naming convention for closures with a truly unique one?" ...
Thread (14 messages)