Skip to content

Commit b57b35c

Browse files
committed
fix(wasm): remove the playground's memory leak and Run budget
- cycle only the request per Run, not the whole PHP module - force Opcache's mmap shared-memory backend on - patch Zend's chunk allocator for Emscripten's whole-mapping-only munmap - restore the ZEND_MM_ERROR allocator diagnostics - recycle the wasm module in the controller as a backstop
1 parent 07ecac0 commit b57b35c

8 files changed

Lines changed: 622 additions & 168 deletions

File tree

documentation/contributing/wasm.md

Lines changed: 290 additions & 38 deletions
Large diffs are not rendered by default.

wasm/build.sh

Lines changed: 47 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#!/usr/bin/env bash
22

33
# Build script for Flow PHP Interactive Playground
4-
# Builds PHP 8.5.8
4+
# Builds PHP 8.5.8 as 64-bit wasm (see WASM64_MODE below) with a forced mmap Opcache shared-memory
5+
# backend (see php_cv_shm_mmap_anon below).
56

67
set -xeu
78

@@ -15,6 +16,13 @@ echo "=== Build started at $(date) ==="
1516
PHP_VERSION=8.5.8
1617
PHP_PATH=php-$PHP_VERSION
1718

19+
# MEMORY64=2 is wasm64 for clang/lld lowered to wasm32 by Binaryen, so PHP gets an 8-byte zend_long
20+
# (pack()'s q/Q/J/P, which Floe and sortBy() need) without requiring Memory64 in visitors' browsers.
21+
# Set via EMCC_CFLAGS, not CFLAGS, which is exported below -- after the dependencies are built.
22+
WASM64_MODE="-sMEMORY64=2"
23+
EM_TARGET=wasm64-emscripten
24+
export EMCC_CFLAGS="$WASM64_MODE"
25+
1826
echo "Build libxml2 for WebAssembly"
1927
LIBXML2_VERSION=2.11.4
2028
LIBXML2_DIR=libxml2-$LIBXML2_VERSION
@@ -48,9 +56,8 @@ if [ ! -d "$LIBXML2_DIR" ]; then
4856
fi
4957

5058
echo "Build libpg_query for WebAssembly"
51-
# Single source of truth is the pg_query extension's own pin, so the playground and the CLI can
52-
# never disagree about which PostgreSQL grammar parses. build.sh already consumes that directory's
53-
# ext/ sources below, so this adds no coupling that did not already exist.
59+
# Read from the pg_query extension's own pin, so the playground and the CLI cannot disagree about
60+
# which PostgreSQL grammar parses.
5461
LIBPG_QUERY_VERSION=$(sed -n 's/^LIBPG_QUERY_VERSION := //p' \
5562
"$PROJECT_ROOT/../src/extension/pg-query-ext/Makefile")
5663

@@ -59,27 +66,22 @@ if [ -z "$LIBPG_QUERY_VERSION" ]; then
5966
exit 1
6067
fi
6168

62-
# Version in the directory name, like libxml2/libzip/php above: a bumped pin then misses the
63-
# guard below and re-clones, instead of silently reusing the previous version's checkout.
64-
# Note this tracks a moving branch, so an upstream branch move still needs a manual rm -rf.
69+
# Version in the directory name so a bumped pin re-clones instead of reusing the old checkout.
70+
# This tracks a moving branch, so an upstream branch move still needs a manual rm -rf.
6571
LIBPG_QUERY_DIR=libpg_query-$LIBPG_QUERY_VERSION
6672
LIBPG_QUERY_INSTALL_DIR="$PROJECT_ROOT/$LIBPG_QUERY_DIR"
6773

68-
# Guard on the built archive rather than the directory, like libzip below: a build that fails
69-
# part way then leaves a tree that a directory check would skip, and the missing archive would
70-
# only surface much later at link time.
74+
# Guard on the built archive, not the directory: a part-way failure leaves a tree that a directory
75+
# check would skip, and the missing archive would only surface at link time.
7176
if [ ! -f "$LIBPG_QUERY_INSTALL_DIR/libpg_query.a" ]; then
7277
rm -rf "$LIBPG_QUERY_DIR"
7378
git clone --depth=1 --branch=$LIBPG_QUERY_VERSION \
7479
https://github.com/pganalyze/libpg_query.git "$LIBPG_QUERY_DIR"
7580
cd $LIBPG_QUERY_DIR
7681

77-
# Build with Emscripten - override CC, AR and ARFLAGS.
78-
# libpg_query 18 keeps AR and ARFLAGS separate (Makefile:71-72), unlike 17's combined
79-
# AR := $(AR) rs, so folding the flags into AR gives `emar rcs rs` -- two operations, which
80-
# llvm-ar rejects. Both have to come from the command line: the Makefile's `?=` never fires,
81-
# because GNU make predefines ARFLAGS=-rv and `?=` only assigns when the origin is `undefined`.
82-
# `rcs` rather than the default `-rv` so the archive still gets its symbol index.
82+
# libpg_query 18 keeps AR and ARFLAGS separate, so folding the flags into AR gives `emar rcs rs`,
83+
# which llvm-ar rejects. Both must come from the command line -- make predefines ARFLAGS, so the
84+
# Makefile's `?=` never fires.
8385
emmake make build -j$(nproc) CC=emcc AR=emar ARFLAGS=rcs
8486

8587
cd $PROJECT_ROOT
@@ -95,15 +97,16 @@ if [ ! -f "$LIBZIP_INSTALL_DIR/lib/libzip.a" ]; then
9597
# First, ensure Emscripten's zlib port is built by triggering a compile
9698
# This downloads and builds zlib to the Emscripten cache
9799
echo "int main(){return 0;}" > /tmp/zlib_test.c
98-
# -flto must match CFLAGS below: emcc caches a separate port build per variant, and this
99-
# script links the lto/ one. Without it only the non-LTO libz.a is produced and the check
100-
# after this block fails.
100+
# -flto must match CFLAGS below: emcc caches a separate port build per variant and this links
101+
# the lto/ one.
101102
emcc -flto -sUSE_ZLIB=1 /tmp/zlib_test.c -o /tmp/zlib_test.js 2>/dev/null || true
102103
rm -f /tmp/zlib_test.c /tmp/zlib_test.js /tmp/zlib_test.wasm
103104

104105
# Get Emscripten cache path and locate zlib
105106
EM_CACHE=$(em-config CACHE)
106-
ZLIB_LIBRARY="$EM_CACHE/sysroot/lib/wasm32-emscripten/lto/libz.a"
107+
# Per-target path: hardcoding wasm32 would find a real file of the wrong architecture, which the
108+
# guard below cannot detect.
109+
ZLIB_LIBRARY="$EM_CACHE/sysroot/lib/$EM_TARGET/lto/libz.a"
107110
ZLIB_INCLUDE_DIR="$EM_CACHE/sysroot/include"
108111

109112
echo "Using zlib from Emscripten cache:"
@@ -121,11 +124,11 @@ if [ ! -f "$LIBZIP_INSTALL_DIR/lib/libzip.a" ]; then
121124
tar xf $LIBZIP_DIR.tar.xz
122125
cd $LIBZIP_DIR
123126

127+
# CMakeCache.txt pins the resolved zlib path and target, so a tree left from another target has
128+
# to be cleared rather than reconfigured.
129+
rm -rf build
124130
mkdir -p build && cd build
125131

126-
# Configure libzip for WebAssembly using CMake
127-
# Provide explicit paths to Emscripten's zlib (from its ports system)
128-
# Disable encryption and optional compression to minimize dependencies
129132
emcmake cmake .. \
130133
-DCMAKE_INSTALL_PREFIX=$LIBZIP_INSTALL_DIR \
131134
-DZLIB_LIBRARY=$ZLIB_LIBRARY \
@@ -161,14 +164,15 @@ fi
161164
echo "Patch php-src"
162165
# Re-runs reuse an already extracted $PHP_PATH, so this has to be idempotent: a cleanly applying
163166
# reverse patch means it is already in place. Anything else is a real failure and set -e ends here.
164-
PHP_PATCH="$PROJECT_ROOT/patches/php-8.5-opcache-unistd.patch"
165167
cd "$PHP_PATH"
166168

167-
if patch -p1 --reverse --dry-run --force --silent <"$PHP_PATCH" >/dev/null 2>&1; then
168-
echo " already applied: $(basename "$PHP_PATCH")"
169-
else
170-
patch -p1 --forward <"$PHP_PATCH"
171-
fi
169+
for PHP_PATCH in "$PROJECT_ROOT"/patches/*.patch; do
170+
if patch -p1 --reverse --dry-run --force --silent <"$PHP_PATCH" >/dev/null 2>&1; then
171+
echo " already applied: $(basename "$PHP_PATCH")"
172+
else
173+
patch -p1 --forward <"$PHP_PATCH"
174+
fi
175+
done
172176

173177
cd "$PROJECT_ROOT"
174178

@@ -191,11 +195,10 @@ cp -r "$SNAPPY_EXT_DIR" "$SNAPPY_EXT_DST"
191195

192196
echo "Configure PHP"
193197

194-
# Use -Oz for size optimization instead of -O3 for speed
195198
# -DHAVE_REALLOCARRAY=1: emcc's link probe for reallocarray() fails while its sysroot header still
196-
# declares the symbol, so main/php_glob.c (always built since 8.5) compiles its own static copy and
197-
# clashes. See https://github.com/php/php-src/issues/19152.
198-
export CFLAGS="-Oz -flto -fPIC -g0 -DZEND_MM_ERROR=0 -DHAVE_REALLOCARRAY=1 -I$LIBXML2_INSTALL_DIR/include/libxml2 -I$LIBPG_QUERY_INSTALL_DIR -I$LIBPG_QUERY_INSTALL_DIR/src -I$LIBZIP_INSTALL_DIR/include -sUSE_ZLIB=1"
199+
# declares it, so main/php_glob.c compiles a clashing static copy (php-src#19152).
200+
# ZEND_MM_ERROR is deliberately NOT 0 -- it gates the messages that named the chunk-alignment leak.
201+
export CFLAGS="-Oz -flto -fPIC -g0 -DHAVE_REALLOCARRAY=1 -I$LIBXML2_INSTALL_DIR/include/libxml2 -I$LIBPG_QUERY_INSTALL_DIR -I$LIBPG_QUERY_INSTALL_DIR/src -I$LIBZIP_INSTALL_DIR/include -sUSE_ZLIB=1"
199202
export CXXFLAGS="-Oz -flto -fPIC -g0 -std=c++11 -sUSE_ZLIB=1"
200203
export LDFLAGS="-L$LIBXML2_INSTALL_DIR/lib -L$LIBPG_QUERY_INSTALL_DIR -L$LIBZIP_INSTALL_DIR/lib -sUSE_ZLIB=1"
201204

@@ -207,13 +210,7 @@ export LIBZIP_LIBS="-L$LIBZIP_INSTALL_DIR/lib -lzip"
207210

208211
cd $PHP_PATH
209212

210-
# Configure with extensions required by Flow PHP
211-
# - bcmath: required by flow-php/parquet
212-
# - libxml: required as base for XML extensions
213-
# - xml, dom, xmlreader, xmlwriter: required by flow-php/etl-adapter-xml
214-
# - phar, mbstring: essential PHP extensions
215-
# - iconv: required by symfony/polyfill-mbstring
216-
# - zip: required by flow-php/etl-adapter-excel (XLSX files are ZIP archives)
213+
# The extension set and the reason for each is in documentation/contributing/wasm.md.
217214

218215
# Fix permissions for build scripts
219216
chmod +x buildconf build/config-stubs build/shtool 2>/dev/null || true
@@ -223,13 +220,14 @@ bash ./buildconf --force
223220

224221
set +e
225222

226-
# --disable-opcache-jit is NOT redundant next to --disable-all. Since php-src 7b4c14dc1016 opcache
227-
# is non-optional and --disable-all cannot reach it, but its JIT is still gated on $host_cpu -- and
228-
# emconfigure passes no --host, so config.guess reports the BUILD machine. On any host autoconf
229-
# calls JIT-capable (x86_64, aarch64, ...) emcc would be handed jit/ir/*.c and dynasm for the host
230-
# ISA to compile into a wasm32 binary. Same class of workaround as --disable-fiber-asm below.
231-
# --disable-huge-code-pages is pointless under wasm and matches the other PHP-to-wasm builds.
232-
# Both pass [no] as their 5th PHP_ARG_ENABLE arg, so neither is affected by --disable-all.
223+
# Opcache's shared-memory probes all call fork(), which Emscripten lacks, so configure would leave
224+
# Opcache permanently inert. Only the fork() half really fails -- Emscripten's mmap() serves
225+
# anonymous MAP_SHARED from linear memory and its fcntl() reports the locks taken, which is enough.
226+
export php_cv_shm_mmap_anon=yes
227+
228+
# --disable-opcache-jit and --disable-huge-code-pages are NOT redundant next to --disable-all: both
229+
# pass [no] as their 5th PHP_ARG_ENABLE arg, so --disable-all cannot reach either. Without the JIT
230+
# flag emcc is handed dynasm for the build machine's ISA, since emconfigure passes no --host.
233231
emconfigure ./configure \
234232
--disable-all \
235233
--disable-opcache-jit \
@@ -290,9 +288,9 @@ echo "Compile pib_eval wrapper"
290288
emcc $CFLAGS -I . -I Zend -I main -I TSRM/ ../pib_eval.c -c -o pib_eval.o
291289

292290
echo "Link everything together"
293-
emcc $CFLAGS $LDFLAGS \
291+
emcc $CFLAGS $LDFLAGS $WASM64_MODE \
294292
-s ENVIRONMENT=web \
295-
-s EXPORTED_FUNCTIONS='["_pib_eval", "_pib_force_exit", "_php_embed_init", "_zend_eval_string", "_php_embed_shutdown"]' \
293+
-s EXPORTED_FUNCTIONS='["_pib_eval", "_pib_force_exit", "_pib_heap_bytes"]' \
296294
-s EXPORTED_RUNTIME_METHODS='["ccall","FS","UTF8ToString","lengthBytesUTF8","stringToUTF8","getValue","setValue","ENV"]' \
297295
-s MODULARIZE=1 \
298296
-s EXPORT_NAME="'PHP'" \
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
Stop Zend's chunk allocator from orphaning ~2 MB of wasm memory per chunk.
2+
3+
zend_mm_chunk_alloc_int() aligns a chunk by over-allocating and then munmap()ing
4+
the unwanted head and tail. Emscripten's munmap() returns EINVAL unless the length
5+
matches a whole mapping, so both trims fail -- and because wasm memory can never
6+
shrink, one ZEND_MM_CHUNK_SIZE is lost per chunk, permanently. That is what gave
7+
the playground a finite Run budget: 2.362 MB leaked per Run, and the tab stopped
8+
executing PHP after 813 Runs. It was silent because build.sh used to compile with
9+
-DZEND_MM_ERROR=0, which suppresses the "munmap() failed" message.
10+
11+
posix_memalign() aligns directly, so there is nothing to trim. chunk_truncate()
12+
and chunk_extend() then report failure, which is the branch _WIN32 already takes
13+
and which callers handle by falling back to allocate-copy-free.
14+
15+
Not reported upstream: php-src does not target Emscripten. Re-check on the next
16+
PHP bump.
17+
18+
--- a/Zend/zend_alloc.c
19+
+++ b/Zend/zend_alloc.c
20+
@@ -477,7 +477,9 @@
21+
#endif
22+
}
23+
24+
-#ifndef HAVE_MREMAP
25+
+/* zend_mm_chunk_extend() is the only non-_WIN32 caller and cannot succeed under Emscripten, so
26+
+ * without this guard the definition is left unreferenced. */
27+
+#if !defined(HAVE_MREMAP) && !defined(__EMSCRIPTEN__)
28+
static void *zend_mm_mmap_fixed(void *addr, size_t size)
29+
{
30+
#ifdef _WIN32
31+
@@ -740,6 +742,18 @@
32+
33+
static void *zend_mm_chunk_alloc_int(size_t size, size_t alignment)
34+
{
35+
+#ifdef __EMSCRIPTEN__
36+
+ /* Emscripten's munmap() cannot release part of a mapping, so the over-allocate-then-trim path
37+
+ * below orphans one ZEND_MM_CHUNK_SIZE per chunk and wasm memory never shrinks. Early return
38+
+ * rather than #else, so the generic path stays referenced and -Wunused-function stays quiet. */
39+
+ void *aligned;
40+
+
41+
+ if (posix_memalign(&aligned, alignment, size) != 0) {
42+
+ return NULL;
43+
+ }
44+
+
45+
+ return aligned;
46+
+#endif
47+
void *ptr = zend_mm_mmap(size);
48+
49+
if (ptr == NULL) {
50+
@@ -818,7 +832,12 @@
51+
return;
52+
}
53+
#endif
54+
+#ifdef __EMSCRIPTEN__
55+
+ /* Paired with the posix_memalign() in zend_mm_chunk_alloc_int(). */
56+
+ free(addr);
57+
+#else
58+
zend_mm_munmap(addr, size);
59+
+#endif
60+
}
61+
62+
static int zend_mm_chunk_truncate(zend_mm_heap *heap, void *addr, size_t old_size, size_t new_size)
63+
@@ -832,10 +851,11 @@
64+
}
65+
}
66+
#endif
67+
-#ifndef _WIN32
68+
+#if !defined(_WIN32) && !defined(__EMSCRIPTEN__)
69+
zend_mm_munmap((char*)addr + new_size, old_size - new_size);
70+
return 1;
71+
#else
72+
+ /* A posix_memalign() block cannot be partially released; callers fall back to copy-and-free. */
73+
return 0;
74+
#endif
75+
}
76+
@@ -860,9 +880,10 @@
77+
/* Sanity check: The mapping shouldn't have moved. */
78+
ZEND_ASSERT(ptr == addr);
79+
return 1;
80+
-#elif !defined(_WIN32)
81+
+#elif !defined(_WIN32) && !defined(__EMSCRIPTEN__)
82+
return (zend_mm_mmap_fixed((char*)addr + old_size, new_size - old_size) != NULL);
83+
#else
84+
+ /* Emscripten's mmap() rejects a non-NULL addr hint, so a fixed extension can never succeed. */
85+
return 0;
86+
#endif
87+
}

wasm/pib_eval.c

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "Zend/zend_interfaces.h"
44
#include "Zend/zend_compile.h"
55
#include <emscripten.h>
6+
#include <emscripten/heap.h>
67
#include <stdlib.h>
78
#include <string.h>
89

@@ -78,7 +79,7 @@ static void pib_report_exception(zend_object *ex) {
7879
zend_string_release(message);
7980
zend_string *file = zval_get_string(GET_PROPERTY_SILENT(&exception, ZEND_STR_FILE));
8081
zend_long line = zval_get_long(GET_PROPERTY_SILENT(&exception, ZEND_STR_LINE));
81-
fprintf(stderr, "At %s:%d\n", ZSTR_VAL(file), line);
82+
fprintf(stderr, "At %s:" ZEND_LONG_FMT "\n", ZSTR_VAL(file), line);
8283
zend_string_release(file);
8384
/*
8485
// Can't get this to work at the end of execution.
@@ -97,20 +98,66 @@ static void pib_report_exception(zend_object *ex) {
9798
}
9899
}
99100

100-
// Based on code by https://github.com/oraoto/pib with modifications.
101-
int EMSCRIPTEN_KEEPALIVE pib_eval(char *code) {
102-
int ret = 0;
103-
// USE_ZEND_ALLOC prevents using fast shutdown.
104-
// putenv("USE_ZEND_ALLOC=0");
105-
php_embed_init(0, NULL);
101+
// One PHP *module* lifecycle per page, one *request* lifecycle per Run.
102+
static int pib_module_started = 0;
103+
104+
// Everything php_embed_init() does after php_request_startup(), plus this file's own per-request
105+
// setup. sapi_activate() resets SG() on every request, so this runs for every request, not only
106+
// the first.
107+
static void pib_request_prepare(void) {
108+
SG(headers_sent) = 1;
109+
SG(request_info).no_headers = 1;
110+
106111
pib_cli_register_file_handles();
107112

108113
// Show fatal E_COMPILE_ERRORs and other errors properly (startup errors are normally hidden)
109-
PG(display_startup_errors)=1;
110-
PG(during_request_startup)=0;
114+
PG(display_startup_errors) = 1;
115+
PG(during_request_startup) = 0;
111116

112117
// Enable error display to stdout
113118
PG(display_errors) = 1;
119+
}
120+
121+
// Opcache's shared memory is allocated once, at MINIT, and sized by opcache.memory_consumption --
122+
// so it has to be set before php_embed_init(). ini_defaults is the SAPI's documented hook for that
123+
// (sapi/embed/php_embed.c:201-215); ini_entries cannot be used because php_embed_init() overwrites
124+
// it with its own HARDCODED_INI at :216.
125+
#define PIB_INI_DEFAULT(name, value) \
126+
ZVAL_NEW_STR(&tmp, zend_string_init(value, sizeof(value) - 1, 1)); \
127+
zend_hash_str_update(configuration_hash, name, sizeof(name) - 1, &tmp);
128+
129+
static void pib_ini_defaults(HashTable *configuration_hash) {
130+
zval tmp;
131+
PIB_INI_DEFAULT("opcache.memory_consumption", "32")
132+
PIB_INI_DEFAULT("opcache.interned_strings_buffer", "8")
133+
PIB_INI_DEFAULT("opcache.validate_timestamps", "0")
134+
}
135+
136+
#undef PIB_INI_DEFAULT
137+
138+
double EMSCRIPTEN_KEEPALIVE pib_heap_bytes(void) {
139+
return (double) emscripten_get_heap_size();
140+
}
141+
142+
// Based on code by https://github.com/oraoto/pib with modifications.
143+
int EMSCRIPTEN_KEEPALIVE pib_eval(char *code) {
144+
int ret = 0;
145+
// USE_ZEND_ALLOC prevents using fast shutdown.
146+
// putenv("USE_ZEND_ALLOC=0");
147+
if (!pib_module_started) {
148+
php_embed_module.ini_defaults = pib_ini_defaults;
149+
150+
// The shipped version ignored this return value, so a failed init fell through into
151+
// zend_first_try on a dead engine and reported nothing at all.
152+
if (php_embed_init(0, NULL) == FAILURE) {
153+
fprintf(stderr, "Fatal error: failed to initialize the PHP engine\n");
154+
fflush(stderr);
155+
return FAILURE;
156+
}
157+
158+
pib_module_started = 1;
159+
pib_request_prepare();
160+
}
114161

115162
zend_first_try {
116163
// Set error_reporting to E_ALL
@@ -148,9 +195,19 @@ int EMSCRIPTEN_KEEPALIVE pib_eval(char *code) {
148195
}
149196
ret = EG(exit_status);
150197
} zend_end_try();
151-
php_embed_shutdown();
198+
199+
php_request_shutdown((void *) 0);
200+
152201
fflush(stdout);
153202
fflush(stderr);
203+
204+
if (php_request_startup() == FAILURE) {
205+
pib_module_started = 0;
206+
return FAILURE;
207+
}
208+
209+
pib_request_prepare();
210+
154211
return ret;
155212
}
156213

0 commit comments

Comments
 (0)