Skip to content

Commit afd721f

Browse files
dktappsdktapps
committed
Fixed closure used variables on PHP 8.5
Windows build is cooked until beta4 is released because we need php/php-src#19886
1 parent 5d9ad4d commit afd721f

File tree

4 files changed

+64
-19
lines changed

4 files changed

+64
-19
lines changed

‎.github/workflows/main-php-matrix-windows.yml‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ on:
77
description: 'PHP version'
88
type: string
99
required: true
10+
opcache-shared:
11+
description: 'Whether OPcache needs a zend_extension option (not needed in 8.5+)'
12+
type: boolean
13+
default: false
1014
vs-arch:
1115
description: 'CPU arch to build for (x86 or x64)'
1216
type: string
@@ -21,7 +25,7 @@ on:
2125
default: 'windows-2022'
2226

2327
env:
24-
PHP_SDK_BINARY_TOOLS_VER: 2.3.0
28+
PHP_SDK_BINARY_TOOLS_VER: 2.4.0
2529
PTHREAD_W32_VER: 3.0.0
2630

2731
jobs:
@@ -35,7 +39,7 @@ jobs:
3539
- name: Set PHP build cache key
3640
id: cache-key
3741
run: |
38-
echo "key=php-debug-${{ inputs.php }}-${{ inputs.vs-arch }}-${{ inputs.vs-crt }}-${{ inputs.runs-on }}" >> $env:GITHUB_OUTPUT
42+
echo "key=php-debug-${{ inputs.php }}-${{ inputs.vs-arch }}-${{ inputs.vs-crt }}-${{ inputs.runs-on }}-php-sdk-${{ env.PHP_SDK_BINARY_TOOLS_VER }}" >> $env:GITHUB_OUTPUT
3943
4044
- name: Set php-sdk task command
4145
id: php-sdk-command
@@ -169,7 +173,7 @@ jobs:
169173
bin
170174
php-sdk
171175
deps
172-
key: php-debug-${{ inputs.php }}-${{ inputs.vs-arch }}-${{ inputs.vs-crt }}-${{ inputs.runs-on }}
176+
key: php-debug-${{ inputs.php }}-${{ inputs.vs-arch }}-${{ inputs.vs-crt }}-${{ inputs.runs-on }}-php-sdk-${{ env.PHP_SDK_BINARY_TOOLS_VER }}
173177

174178
- name: Set php-sdk task command
175179
id: php-sdk-command
@@ -204,7 +208,11 @@ jobs:
204208
$opcache = "${{ matrix.opcache }}"
205209
if ($opcache -ne "off") {
206210
echo "Enabling OPcache"
207-
echo "zend_extension=php_opcache.dll" >> php.ini
211+
$opcache_shared = "${{ inputs.opcache-shared }}"
212+
if ($opcache_shared -eq "true") {
213+
echo "OPcache is a shared extension in this version"
214+
echo "zend_extension=php_opcache.dll" >> php.ini
215+
}
208216
echo "opcache.enable=1" >> php.ini
209217
echo "opcache.enable_cli=1" >> php.ini
210218
echo "opcache.protect_memory=1" >> php.ini

‎.github/workflows/main-php-matrix.yml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ on:
77
description: 'PHP version'
88
required: true
99
type: string
10+
opcache-shared:
11+
description: 'Whether opcache needs a zend_extension option (not needed in 8.5+)'
12+
type: boolean
13+
default: false
1014
runs-on:
1115
description: 'GitHub runner'
1216
default: 'ubuntu-22.04'
@@ -122,7 +126,10 @@ jobs:
122126
echo "extension=pmmpthread.so" > $GITHUB_WORKSPACE/php.ini
123127
if [[ "${{ matrix.opcache }}" != "off" ]]; then
124128
echo "Enabling OPcache"
125-
echo "zend_extension=opcache.so" >> $GITHUB_WORKSPACE/php.ini
129+
if [[ "${{ inputs.opcache-shared }}" == "true" ]]; then
130+
echo "OPcache is a shared extension in this version"
131+
echo "zend_extension=opcache.so" >> $GITHUB_WORKSPACE/php.ini
132+
fi
126133
echo "opcache.enable=1" >> $GITHUB_WORKSPACE/php.ini
127134
echo "opcache.enable_cli=1" >> $GITHUB_WORKSPACE/php.ini
128135
echo "opcache.protect_memory=1" >> $GITHUB_WORKSPACE/php.ini

‎.github/workflows/main.yml‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,17 @@ jobs:
1111
strategy:
1212
fail-fast: false
1313
matrix:
14-
php:
15-
- 8.1.33
16-
- 8.2.29
17-
- 8.3.25
18-
- 8.4.12
14+
include:
15+
- php: 8.1.33
16+
opcache-shared: true
17+
- php: 8.2.29
18+
opcache-shared: true
19+
- php: 8.3.25
20+
opcache-shared: true
21+
- php: 8.4.12
22+
opcache-shared: true
23+
- php: 8.5.0beta3
24+
opcache-shared: false
1925

2026
uses: ./.github/workflows/main-php-matrix.yml
2127
with:
@@ -30,12 +36,19 @@ jobs:
3036
include:
3137
- php: 8.1.33
3238
vs-crt: vs16
39+
opcache-shared: true
3340
- php: 8.2.29
3441
vs-crt: vs16
42+
opcache-shared: true
3543
- php: 8.3.25
3644
vs-crt: vs16
45+
opcache-shared: true
3746
- php: 8.4.12
3847
vs-crt: vs17
48+
opcache-shared: true
49+
- php: 8.5.0beta3
50+
vs-crt: vs17
51+
opcache-shared: false
3952

4053
uses: ./.github/workflows/main-php-matrix-windows.yml
4154
with:

‎src/copy.c‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -787,12 +787,29 @@ zend_result pmmpthread_copy_closure(const pmmpthread_ident_t* owner, zend_closur
787787
);
788788
} else {
789789
HashTable* static_variables = NULL;
790-
if (closure_obj->func.type == ZEND_USER_FUNCTION && closure_obj->func.op_array.static_variables != NULL) {
791-
//if this is a real closure, we need to update the static_variables from the original closure object
792-
//so that the copied closure has the correct use()d variables
793-
//this should not modify the original closure
794-
//these have to be copied before the closure is created, to maintain the original behaviour
795-
static_variables = pmmpthread_copy_statics(owner, closure_obj->func.op_array.static_variables);
790+
#if PHP_VERSION_ID >= 80200
791+
HashTable* static_variables_ptr = NULL;
792+
#endif
793+
if (closure_obj->func.type == ZEND_USER_FUNCTION){
794+
if (closure_obj->func.op_array.static_variables != NULL) {
795+
//if this is a real closure, we need to update the static_variables from the original closure object
796+
//so that the copied closure has the correct use()d variables
797+
//this should not modify the original closure
798+
//these have to be copied before the closure is created, to maintain the original behaviour
799+
static_variables = pmmpthread_copy_statics(owner, closure_obj->func.op_array.static_variables);
800+
}
801+
#if PHP_VERSION_ID >= 80200
802+
//in PHP 8.5, the static_variables field is not overwritten when creating a new closure, so it may be different
803+
//than the static_variables_ptr, which will typically contain bound variables
804+
HashTable *origin_static_variables_ptr = ZEND_MAP_PTR_GET(closure_obj->func.op_array.static_variables_ptr);
805+
if (origin_static_variables_ptr != NULL) {
806+
if (origin_static_variables_ptr != closure_obj->func.op_array.static_variables) {
807+
static_variables_ptr = pmmpthread_copy_statics(owner, origin_static_variables_ptr);
808+
} else {
809+
static_variables_ptr = static_variables;
810+
}
811+
}
812+
#endif
796813
}
797814

798815
//we don't know where the definition for this closure is, so create a definition from this copy of it
@@ -810,15 +827,15 @@ zend_result pmmpthread_copy_closure(const pmmpthread_ident_t* owner, zend_closur
810827
if (static_variables != NULL) {
811828
zend_closure* new_closure = (zend_closure*)Z_OBJ_P(pzval);
812829
ZEND_ASSERT(new_closure->func.type == ZEND_USER_FUNCTION);
813-
if (new_closure->func.op_array.static_variables != NULL) {
830+
if (ZEND_MAP_PTR_GET(new_closure->func.op_array.static_variables_ptr) != NULL) {
814831
//the closure may have static_variables allocated from its original creation by zend_compile.c
815-
zend_array_release(new_closure->func.op_array.static_variables);
832+
zend_array_release(ZEND_MAP_PTR_GET(new_closure->func.op_array.static_variables_ptr));
816833
}
817834
new_closure->func.op_array.static_variables = static_variables;
818835
#if PHP_VERSION_ID >= 80200
819836
ZEND_MAP_PTR_INIT(
820837
new_closure->func.op_array.static_variables_ptr,
821-
new_closure->func.op_array.static_variables
838+
static_variables_ptr
822839
);
823840
#else
824841
//this is not strictly necessary for 8.1, but is here for the sake of completeness

0 commit comments

Comments
 (0)