fix(wx): thread-safe wxString for the AsyncLoad fan-out — CvPcb-open trap solved

The ~1/9 wasm trap on CvPcb open ("index out of bounds" / "indirect call to
null" in a footprint AsyncLoad pool worker, then eeschema aborting on the
broken future — and the eeschema-fp-selector "CI-only" trap family, which was
never llvmpipe-specific) was wxString's UTF-8 build mutating SHARED strings on
read-only access from concurrent pool workers: every iterator ctor/dtor
spliced an intrusive list inside the string object, and torn splices wrote
through dead node pointers into other threads' stack frames. Second defect:
the UTF-8 position cache returned stale offsets when another thread's string
died and its address was reused.

Fixed in the wxwidgets fork (per-thread iterator registry + position cache
disabled under Emscripten) — kicad is untouched and AsyncLoad keeps its full
multi-worker fan-out. Falsified along the way (all perturbation masks, not
fixes): serializing the items, mimalloc vs dlmalloc, pthread stack size,
ASYNCIFY_STACK_SIZE, private-copy EnumFromStr, hot-path logging.

New red-first standalone app tests/apps/standalone/wxstring-mt (+ spec
coroutine-wxstring-mt.spec.ts, wx-chromium + coroutine-firefox): shared-string
compares alternating with wide-literal conversions reproduce the exact editor
trap signatures on the unfixed wx and run 4.7M rounds clean on the fixed one;
the pos-cache address-reuse dance corrupts on the first reuse before and
survives 673 after; two guard modes keep the iterator fix-up feature honest
(incl. an anti-elision liveness check — balanced register/unregister pairs in
tight loops can legally be optimized away, so a naive red test tests nothing).

Verification: wx+coroutine suites 382 passed; in-app AsyncLoad hammer 3x1000
rounds clean (baseline died <10); eeschema-assign-footprints spec 20/20
firefox + 20/20 chromium on the final build; lint:determinism clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rb9jsqtHsC3tHTaJ45244j
This commit is contained in:
Viktor Vaczi 2026-07-20 11:56:20 +02:00
commit d538b73557
5 changed files with 580 additions and 9 deletions

View file

@ -611,6 +611,19 @@ raytrace-threads: $(S)/raytrace-threads/raytrace_threads_test.html
# Include the raytracer-threading repro in the default `all` build (prereqs accumulate).
all: $(S)/raytrace-threads/raytrace_threads_test.html
# wxString UTF-8 multithreading red/green tests (shared-iterator registry,
# position cache, iterator fix-up) — see the app header for the modes.
$(S)/wxstring-mt/wxstring_mt_test.o: $(S)/wxstring-mt/wxstring_mt_test.cpp
$(CXX) -c $(CXXFLAGS) -pthread $< -o $@
$(S)/wxstring-mt/wxstring_mt_test.html: $(S)/wxstring-mt/wxstring_mt_test.o $(WX_CORE_LIB) $(JS_FILES)
$(CXX) $< $(LDFLAGS_PTHREAD) --pre-js $(JS) --shell-file $(HTML) -o $@
wxstring-mt: $(S)/wxstring-mt/wxstring_mt_test.html
.PHONY: wxstring-mt
all: $(S)/wxstring-mt/wxstring_mt_test.html
# Un-shimmed copy of KiCad's BS::thread_pool: the single-thread #ifdef __EMSCRIPTEN__ shim in
# detach_task is disabled (-> #if 0) so the pool tests exercise REAL multithreading WITHOUT
# modifying the pristine KiCad header and WITHOUT a compile macro. Regenerated at parse time to

View file

@ -0,0 +1,503 @@
/**
* wxstring_mt_test.cpp red/green tests for wxString's UTF-8 build under
* threads (wxUSE_UNICODE_UTF8).
*
* The UTF-8 wxString keeps two pieces of bookkeeping that are updated on
* read-only access:
* - an intrusive list of live iterators (used to fix iterators up when a
* width-changing in-place edit shifts the byte buffer), historically
* stored INSIDE each string object so iterating a string SHARED between
* threads mutated the shared object without synchronization;
* - a per-thread position cache mapping {string address -> char index ->
* byte offset}, whose entries can outlive a string destroyed by another
* thread and mis-describe a new string reusing the same address.
*
* Modes (?m= / #m= note: `npx serve` cleanUrls drops ?query, use the hash):
* 0 SHARED-ITER many threads iterate ONE shared const wxString, holding
* iterators across an opaque call so the registration writes
* cannot be optimized away. RED (traps / wrong derefs) while
* registration lives in the shared string; GREEN with the
* per-thread registry.
* 1 POSCACHE cross-thread destroy + same-address realloc stale-hit
* dance. RED while the position cache is enabled under
* threads; GREEN with it disabled (or redesigned).
* 2 ITER-FIXUP the feature the iterator registry exists for: in-place
* character assignment that CHANGES the UTF-8 width must fix
* up all live iterators of the same thread. Must be GREEN
* both before and after any registry change.
*
* Console contract (asserted by tests/e2e/wxstring-mt.spec.ts):
* [WXSTR] START mode=.. threads=..
* [WXSTR] SUCCESS mode=.. rounds=..
* [WXSTR] CORRUPT ... (verified wrong value deterministic detection;
* heap corruption may also surface as a trap)
*/
#include "wx/wx.h"
#include <atomic>
#include <chrono>
#include <cstdarg>
#include <cstdio>
#include <thread>
#include <vector>
#ifdef __EMSCRIPTEN__
#include <emscripten/emscripten.h>
#endif
using clk = std::chrono::steady_clock;
static void plog( const char* fmt, ... )
{
char buf[512];
va_list ap;
va_start( ap, fmt );
vsnprintf( buf, sizeof( buf ), fmt, ap );
va_end( ap );
#ifdef __EMSCRIPTEN__
EM_ASM( { console.log( UTF8ToString( $0 ) ); }, buf );
#else
printf( "%s\n", buf );
#endif
}
static int readMode()
{
#ifdef __EMSCRIPTEN__
return EM_ASM_INT( {
var raw = location.search ? location.search.slice( 1 ) : location.hash.slice( 1 );
var v = parseInt( new URLSearchParams( raw ).get( 'm' ), 10 );
return isNaN( v ) ? 0 : v;
} );
#else
return 0;
#endif
}
static long ms( clk::time_point t0 )
{
return (long) std::chrono::duration_cast<std::chrono::milliseconds>( clk::now() - t0 ).count();
}
static std::atomic<bool> g_corrupt{ false };
static void corrupt( const char* fmt, ... )
{
char buf[400];
va_list ap;
va_start( ap, fmt );
vsnprintf( buf, sizeof( buf ), fmt, ap );
va_end( ap );
plog( "[WXSTR] CORRUPT %s", buf );
g_corrupt.store( true );
}
// Opaque boundary: calls through a volatile function pointer cannot be inlined
// or reasoned about, so iterator registration state is observable across them
// and the compiler cannot elide balanced register/unregister pairs (a naive
// tight loop CAN legally be elided — a red test must not rely on one).
static std::atomic<unsigned> g_sink{ 0 };
static void touchIterImpl( wxString::const_iterator& it )
{
g_sink.fetch_add( ( *it ).GetValue(), std::memory_order_relaxed );
}
typedef void ( *TouchIterFn )( wxString::const_iterator& );
static volatile TouchIterFn g_touchIter = touchIterImpl;
// ---------------------------------------------------------------------------
// mode 0 — concurrent iteration of ONE shared string
// ---------------------------------------------------------------------------
static int modeSharedIter( int seconds )
{
// Multibyte content so UTF-8 iteration does real decoding work; the
// expected code points are checked on every deref.
wxString shared;
for( int i = 0; i < 48; ++i )
shared += wxUniChar( 0x3B1 + ( i % 24 ) ); // α..ω repeating
const size_t nThreads =
std::max( 4u, std::thread::hardware_concurrency() );
std::atomic<bool> stop{ false };
std::atomic<long> rounds{ 0 };
std::vector<std::thread> threads;
for( size_t t = 0; t < nThreads; ++t )
{
threads.emplace_back( [&, t]()
{
size_t off = t % 8;
while( !stop.load( std::memory_order_relaxed ) )
{
// Several iterators alive at once, each surviving an opaque
// call: every construction/copy/destruction updates the
// iterator registry.
wxString::const_iterator a = shared.begin();
g_touchIter( a );
wxString::const_iterator b = a;
for( size_t s = 0; s < 8 + off; ++s )
++b;
g_touchIter( b );
wxString::const_iterator c = b;
++c;
g_touchIter( c );
const unsigned got = ( *b ).GetValue();
const unsigned want = 0x3B1 + ( ( 8 + off ) % 24 );
if( got != want )
{
corrupt( "mode=0 deref idx=%zu got=%#x want=%#x round=%ld",
8 + off, got, want, rounds.load() );
stop.store( true );
return;
}
off = ( off + 1 ) % 8;
rounds.fetch_add( 1, std::memory_order_relaxed );
}
} );
}
clk::time_point t0 = clk::now();
while( ms( t0 ) < seconds * 1000 && !g_corrupt.load() )
{
#ifdef __EMSCRIPTEN__
emscripten_sleep( 20 );
#endif
}
stop.store( true );
for( auto& th : threads )
th.join();
return (int) rounds.load();
}
// ---------------------------------------------------------------------------
// mode 1 — position-cache stale hit via cross-thread destroy + address reuse
// ---------------------------------------------------------------------------
static int modePosCache( int seconds )
{
wxString longStr, shortStr;
for( int i = 0; i < 64; ++i )
longStr += wxUniChar( 0x3B1 + ( i % 24 ) );
for( int i = 0; i < 8; ++i )
shortStr += wxUniChar( 0x3B1 + ( i % 24 ) );
std::atomic<wxString*> slot{ nullptr };
std::atomic<int> phase{ 0 };
std::atomic<bool> stop{ false };
std::atomic<long> readerRounds{ 0 };
std::atomic<int> inPass{ 0 }; // reader is inside an indexed pass
std::thread reader( [&]()
{
while( !stop.load( std::memory_order_acquire ) )
{
if( phase.load( std::memory_order_acquire ) != 0 )
continue;
wxString* s = slot.load( std::memory_order_acquire );
if( !s )
continue;
// The controller never frees the published string while a pass is
// in flight (it waits for inPass == 0 after retracting the slot),
// so every read below is of a LIVE string — a wrong value can only
// come from stale cached positions, not from use-after-free.
inPass.store( 1, std::memory_order_release );
if( phase.load( std::memory_order_acquire ) != 0
|| slot.load( std::memory_order_acquire ) != s )
{
inPass.store( 0, std::memory_order_release );
continue;
}
// Indexed reads deep into the string populate THIS thread's
// position cache with (string address -> byte offset) entries.
size_t len = s->length();
for( size_t i = len / 2; i < len; ++i )
{
unsigned got = ( *s )[i].GetValue();
unsigned want = 0x3B1 + ( (int) i % 24 );
if( got != want )
{
corrupt( "mode=1 stale read: len=%zu idx=%zu got=%#x want=%#x",
len, i, got, want );
stop.store( true );
inPass.store( 0, std::memory_order_release );
return;
}
}
readerRounds.fetch_add( 1, std::memory_order_relaxed );
inPass.store( 0, std::memory_order_release );
}
} );
clk::time_point t0 = clk::now();
int swaps = 0, reuse = 0;
bool useLong = true;
while( ms( t0 ) < seconds * 1000 && !g_corrupt.load() )
{
wxString* cur = new wxString( useLong ? longStr : shortStr );
void* prevAddr = (void*) cur;
slot.store( cur, std::memory_order_release );
phase.store( 0, std::memory_order_release );
#ifdef __EMSCRIPTEN__
emscripten_sleep( 5 );
#endif
// Retract, destroy on THIS thread (only this thread's cache entries
// are invalidated), reallocate immediately: same-size classes make the
// allocator hand the address back, and the reader's stale entry now
// describes a DIFFERENT string.
phase.store( 1, std::memory_order_release );
slot.store( nullptr, std::memory_order_release );
while( inPass.load( std::memory_order_acquire ) != 0 )
{
#ifdef __EMSCRIPTEN__
emscripten_sleep( 1 );
#endif
}
delete cur;
useLong = !useLong;
wxString* next = new wxString( useLong ? longStr : shortStr );
if( (void*) next == prevAddr )
++reuse;
slot.store( next, std::memory_order_release );
phase.store( 0, std::memory_order_release );
++swaps;
#ifdef __EMSCRIPTEN__
emscripten_sleep( 5 );
#endif
phase.store( 1, std::memory_order_release );
slot.store( nullptr, std::memory_order_release );
while( inPass.load( std::memory_order_acquire ) != 0 )
{
#ifdef __EMSCRIPTEN__
emscripten_sleep( 1 );
#endif
}
delete next;
}
stop.store( true );
reader.join();
plog( "[WXSTR] poscache swaps=%d addrReuse=%d readerRounds=%ld",
swaps, reuse, readerRounds.load() );
return swaps;
}
// ---------------------------------------------------------------------------
// mode 2 — iterator fix-up across width-changing in-place edits (the feature
// the registry serves; single-threaded, must ALWAYS pass)
// ---------------------------------------------------------------------------
static int modeIterFixup()
{
int checks = 0;
for( int pass = 0; pass < 200; ++pass )
{
wxString s = wxString::FromUTF8( "abcdefghij" );
wxString::iterator i2 = s.begin() + 2; // 'c', before the edit
wxString::iterator i7 = s.begin() + 7; // 'h', after the edit
wxString::const_iterator c9 = ( (const wxString&) s ).begin() + 9; // 'j'
// 'e' (1 byte) -> α (2 bytes): width change forces a byte-shifting
// replace, which must fix up every live iterator of this thread.
s[4] = wxUniChar( 0x3B1 );
if( ( *i2 ).GetValue() != 'c' || ( *i7 ).GetValue() != 'h'
|| ( *c9 ).GetValue() != 'j' )
{
corrupt( "mode=2 grow fixup: got %#x %#x %#x",
( *i2 ).GetValue(), ( *i7 ).GetValue(), ( *c9 ).GetValue() );
return checks;
}
// α (2 bytes) -> 'e' (1 byte): the shrinking direction.
s[4] = wxUniChar( 'e' );
if( ( *i2 ).GetValue() != 'c' || ( *i7 ).GetValue() != 'h'
|| ( *c9 ).GetValue() != 'j' )
{
corrupt( "mode=2 shrink fixup: got %#x %#x %#x",
( *i2 ).GetValue(), ( *i7 ).GetValue(), ( *c9 ).GetValue() );
return checks;
}
checks += 6;
}
return checks;
}
// ---------------------------------------------------------------------------
// mode 3 — registration liveness for the mode-0 iterator pattern: an iterator
// created and held across the same opaque call must be FIXED UP by a
// width-changing edit — which can only happen if it was registered. Guards
// against the mode-0 red test silently testing nothing (optimizer elision).
// ---------------------------------------------------------------------------
static int modeRegistrationLive()
{
int checks = 0;
for( int pass = 0; pass < 100; ++pass )
{
wxString s = wxString::FromUTF8( "abcdefghij" );
wxString::const_iterator a = ( (const wxString&) s ).begin();
g_touchIter( a );
wxString::const_iterator b = a;
for( int i = 0; i < 7; ++i )
++b;
g_touchIter( b ); // 'h', held across the edit below
s[2] = wxUniChar( 0x3B2 ); // 'c' -> β: width change shifts bytes
if( ( *b ).GetValue() != 'h' )
{
corrupt( "mode=3 iterator NOT fixed up (got %#x) — registration "
"was elided; mode 0 would be vacuous", ( *b ).GetValue() );
return checks;
}
checks++;
}
return checks;
}
// ---------------------------------------------------------------------------
// mode 4 — shared-string compares alternating with wide-literal conversions.
// CmpNoCase on strings SHARED between threads registers iterator nodes (which
// live on the caller's stack) in the shared string's intrusive list; a torn
// concurrent splice leaves another thread holding a pointer to a node that has
// since died, and its late m_prev write lands in whatever now occupies that
// stack slot. The victim here is deliberate: immediately after the compares,
// the same frame region holds the conversion temporaries of a
// wchar_t* -> wxString construction (vtable-carrying wxMBConv temp) — a stale
// write corrupts its vptr and the virtual dispatch traps. RED while the
// iterator registry lives inside the shared string; GREEN with a per-thread
// registry.
// ---------------------------------------------------------------------------
static int modeCmpThenConvert( int seconds )
{
std::vector<wxString> names;
for( int i = 0; i < 6; ++i )
{
wxString n;
n << wxS( "Type_" );
for( int c = 0; c < 6 + i; ++c )
n += wxUniChar( 0x3B1 + ( ( i + c ) % 24 ) );
names.push_back( n );
}
const wxString probe = names[3]; // deep copy; compares still iterate BOTH
static const wchar_t* const wideLits[] = {
L"NESTED_TABLE_\u03b1\u03b2", L"PCBJAM_FP_\u03b3\u03b4", L"KiCad_\u03b5\u03b6",
};
const size_t nThreads = std::max( 4u, std::thread::hardware_concurrency() );
std::atomic<bool> stop{ false };
std::atomic<long> rounds{ 0 };
std::vector<std::thread> threads;
for( size_t t = 0; t < nThreads; ++t )
{
threads.emplace_back( [&, t]()
{
size_t k = t;
while( !stop.load( std::memory_order_relaxed ) )
{
// EnumFromStr shape: compare the shared probe against every
// shared registry name (iterator nodes on THIS stack,
// registered in the SHARED strings)...
int hits = 0;
for( const wxString& n : names )
{
if( n.CmpNoCase( probe ) == 0 )
++hits;
}
// ...then immediately build wxStrings from wide literals in
// the same stack region (the conversion path with the
// vtable-carrying conv temporary).
const wchar_t* lit = wideLits[k % 3];
wxString conv( lit );
++k;
if( hits != 1 || conv.empty() )
{
corrupt( "mode=4 hits=%d convLen=%zu round=%ld",
hits, (size_t) conv.length(), rounds.load() );
stop.store( true );
return;
}
rounds.fetch_add( 1, std::memory_order_relaxed );
}
} );
}
clk::time_point t0 = clk::now();
while( ms( t0 ) < seconds * 1000 && !g_corrupt.load() )
{
#ifdef __EMSCRIPTEN__
emscripten_sleep( 20 );
#endif
}
stop.store( true );
for( auto& th : threads )
th.join();
return (int) rounds.load();
}
// ---------------------------------------------------------------------------
class WxStrFrame : public wxFrame
{
public:
WxStrFrame() : wxFrame( nullptr, wxID_ANY, wxS( "wxstring-mt" ), wxDefaultPosition,
wxSize( 320, 120 ) )
{
}
};
class WxStrApp : public wxApp
{
public:
bool OnInit() override
{
const int mode = readMode();
plog( "[WXSTR] START mode=%d threads=%u", mode,
std::thread::hardware_concurrency() );
clk::time_point t0 = clk::now();
int rounds = 0;
const int seconds = 15;
switch( mode )
{
case 0: rounds = modeSharedIter( seconds ); break;
case 1: rounds = modePosCache( seconds ); break;
case 2: rounds = modeIterFixup(); break;
case 3: rounds = modeRegistrationLive(); break;
case 4: rounds = modeCmpThenConvert( seconds ); break;
default: plog( "[WXSTR] unknown mode=%d", mode ); break;
}
if( !g_corrupt.load() )
plog( "[WXSTR] SUCCESS mode=%d rounds=%d totalMs=%ld", mode, rounds, ms( t0 ) );
( new WxStrFrame() )->Show();
return true;
}
};
wxIMPLEMENT_APP( WxStrApp );

View file

@ -0,0 +1,53 @@
import { test, expect } from './utils/fixtures';
// wxString UTF-8 build under threads (tests/apps/standalone/wxstring-mt).
//
// The UTF-8 wxString updates bookkeeping on read-only access: an intrusive
// registry of live iterators (per-THREAD lists since the wasm-port fix in
// wxwidgets include/wx/string.h — historically a list inside each string
// object, which made concurrent reads of a SHARED string race), and a
// per-thread position cache (disabled under Emscripten — its entries could
// outlive a string destroyed by another thread and mis-describe a new string
// at a reused address). These modes were built red-first against stock wx:
// modes 1 and 4 trapped/corrupted (mode 4 with the exact editor trap
// signatures: "index out of bounds" / "indirect call to null"), and turned
// green with the per-thread registry + disabled cache. Modes 0/2/3 guard the
// surrounding behavior (mode 2/3: the iterator fix-up feature the registry
// exists for must keep working; mode 3 also proves the registration writes
// are not optimized away — without it, mode 0/4 could silently test nothing).
//
// Named coroutine-* so playwright's coroutine-firefox project runs it on real
// Firefox in addition to wx-chromium (pthread app; WebKit is skipped).
const APP = '/standalone/wxstring-mt/wxstring_mt_test.html';
const MODES: { m: number; name: string; minRounds: number }[] = [
{ m: 0, name: 'concurrent iteration of one shared string', minRounds: 10000 },
{ m: 1, name: 'position-cache cross-thread destroy + address reuse', minRounds: 10 },
{ m: 2, name: 'iterator fix-up across width-changing edits', minRounds: 1000 },
{ m: 3, name: 'registration liveness (fix-up through opaque calls)', minRounds: 100 },
{ m: 4, name: 'shared-string compares + wide-literal conversions', minRounds: 10000 },
];
async function waitForLog( testLogger: { consoleLogs: string[] }, needle: string, timeout = 60000 ) {
await expect.poll( () => testLogger.consoleLogs.some( l => l.includes( needle ) ), { timeout } ).toBe( true );
}
test.describe( 'wxString UTF-8 multithreading (per-thread iterator registry, pos cache off)', () => {
for( const { m, name, minRounds } of MODES ) {
test( `mode ${m}: ${name}`, async ( { page, testLogger } ) => {
await page.goto( `${APP}#m=${m}` );
await waitForLog( testLogger, `[WXSTR] SUCCESS mode=${m}` );
const line = testLogger.consoleLogs.find( l => l.includes( `[WXSTR] SUCCESS mode=${m}` ) )!;
const rounds = +( line.match( /rounds=(\d+)/ )?.[1] ?? -1 );
expect( rounds, 'the mode must have done real work' ).toBeGreaterThanOrEqual( minRounds );
expect( testLogger.consoleLogs.filter( l => l.includes( '[WXSTR] CORRUPT' ) ),
'no verified corruption' ).toHaveLength( 0 );
expect( testLogger.errors.filter( e => !e.includes( 'favicon' ) ),
'no runtime errors' ).toHaveLength( 0 );
} );
}
} );

View file

@ -131,14 +131,16 @@ test('Tools → Assign Footprints opens CvPcb (merged third kiface)', async ({ p
}
}, item);
// TODO(cvpcb-open-trap): ~1/9 local web-firefox runs die right here — the
// EVT.MENU dispatch traps ("wx_dom_event(48,9) failed" + worker
// "RuntimeError: index out of bounds"), eeschema survives, CvPcb never
// appears, zero libs-bridge calls. Same trap signature as the
// eeschema-fp-selector CI trap (docs/features/web-e2e-rot/01) — which
// reproduced on macOS real GL, so it is NOT llvmpipe-only. Needs a
// debug-symbol repro; post-link binaryen rewriting makes the shipped
// module's DWARF useless for symbolizing the trap offset.
// The ~1/9 "index out of bounds" trap that used to fire right here (a pool
// worker dying in the footprint AsyncLoad items, then eeschema's EVT.MENU
// dispatch aborting on the broken future) was root-caused to wxString's
// UTF-8 build mutating SHARED strings on read-only access from the
// concurrent workers — fixed in the wx fork (per-thread iterator registry +
// position cache disabled under Emscripten, include/wx/string.h), with the
// red/green repro in tests/apps/standalone/wxstring-mt. The
// eeschema-fp-selector "CI-only" trap (docs/features/web-e2e-rot/01) is the
// same family (it reproduced on macOS real GL, so it was never
// llvmpipe-specific).
//
// The frame construction + netlist mail runs off wxPostEvent'd follow-ups,
// which the wx WASM port only flushes on input events — wiggle the mouse

@ -1 +1 @@
Subproject commit 4722e07ef1a45cb31201ed8159051634ffc524f0
Subproject commit a61bcf4aa8684bf2d3afa9d30b3e0edfb7debec1