Skip to content

Never give the end of a full pool to an empty string - #1338

Merged
uNetworkingAB merged 1 commit into
uNetworking:masterfrom
nigrosimone:nativestring-full-pool
Sep 30, 2026
Merged

uNetworkingAB merged 1 commit into
uNetworking:masterfrom
nigrosimone:nativestring-full-pool

Conversation

@nigrosimone

@nigrosimone nigrosimone commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

When a first string fills the 128 KB pool exactly and the next string is empty, alloc(0) returns pool.data() + pool.size(). The destructor sees a pointer that is not inside the pool and passes it to ::free, so the process crashes. For example app.publish('x'.repeat(128 * 1024), ''), or res.writeHeader(key, '') with a key of that size. It happens also on v20.71.0: 0xC0000374 on Windows, bad-free with ASAN on Linux.

Fix: alloc goes to malloc also when the allocation reaches the end of the pool, so a pointer is always inside the pool or from malloc.

Test: tests/strings.js does the publish above, it crashes without the fix.

@nigrosimone
nigrosimone marked this pull request as ready for review September 30, 2026 18:24
@github-actions

Copy link
Copy Markdown

Benchmark: ea157aa against f4fc521

Scenario Base req/s Head req/s Head / base Rounds Noise Busy CPU per request
http/hello-world 110019 109113 0.991x (-0.9%) 0.99 to 0.99 ±0.8% 100% / 100% 9.08 / 9.14 us (1.008x)
http/headers 94049 94431 1.005x (+0.5%) 1.00 to 1.01 ±1.2% 100% / 100% 10.62 / 10.58 us (0.996x)
http/json-post 85322 85898 1.010x (+1.0%) 1.00 to 1.02 ±1.7% 100% / 100% 11.71 / 11.63 us (0.990x)
http/cached 122427 122711 1.002x (+0.2%) 1.00 to 1.01 ±0.4% 100% / 100% 8.16 / 8.15 us (0.998x)
ws/echo-20b 121927 122287 1.003x (+0.3%) 1.00 to 1.00 ±0.9% 100% / 100% 8.19 / 8.17 us (0.997x)
ws/echo-4kb 107568 107852 1.003x (+0.3%) 1.00 to 1.01 ±1.3% 100% / 100% 9.29 / 9.26 us (0.997x)

4 rounds of 3s per scenario, each round loading base, head and a second process of each one, from a copy of its build, after the other, in an order that changes every round. "Head / base" is the median of the per-round ratios of req/s and "Rounds" their range. "Noise" is how far base/base and head/head, the same code on both sides, got from 1 in this same run: that is what the machine did, so a row is marked only when the median moved further than that, at least 2%, and every round moved the same way: 👀 slower, 🏆 faster. "Busy" is the cpu time of the server process over the round and "CPU per request" that time divided by the requests it answered, with the head/base ratio judged against its own same-code band and bold when it cleared it. When busy is well under 100% the load generator set the pace, the req/s are its and the cpu per request is the column to read. Only the ratios are comparable across runs, the absolute figures depend on the runner.

Node v26.10.0, AMD EPYC 9V74 80-Core Processor, 4 cores (server on cpu 0, load on 2,3), http_load_test and load_test with 100 connections.

@uNetworkingAB
uNetworkingAB merged commit 55d2cc3 into uNetworking:master Sep 30, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants