From ad5c4f6772bfd679ebfc9fa26d998d7f2504971a Mon Sep 17 00:00:00 2001 From: Viktor Vaczi Date: Mon, 15 Dec 2025 11:47:50 +0100 Subject: [PATCH] Fix pthread deadlock by using dynamic PTHREAD_POOL_SIZE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The KiCad WASM build was freezing during initialization due to a pthread deadlock. When hardware_concurrency() > PTHREAD_POOL_SIZE, new Web Worker creation requires the event loop, but the main thread busy-waits blocking it. Changes: - Use navigator.hardwareConcurrency for PTHREAD_POOL_SIZE in KiCad build - Add threadpool test to reproduce and verify the fix - Add single-target build support to build-wasm-test.sh - Default docker/build.sh to -j 10 for faster builds 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- docker/build.sh | 8 +- scripts/build-wasm-test.sh | 82 +++++++--- scripts/kicad/build-pcbnew.sh | 2 +- tests/e2e/threadpool.spec.ts | 89 +++++++++++ tests/wasm-app/Makefile.wasm | 20 ++- .../standalone/threadpool/threadpool_test.cpp | 146 ++++++++++++++++++ 6 files changed, 322 insertions(+), 25 deletions(-) create mode 100644 tests/e2e/threadpool.spec.ts create mode 100644 tests/wasm-app/standalone/threadpool/threadpool_test.cpp diff --git a/docker/build.sh b/docker/build.sh index e9d3fe2..7b818fb 100755 --- a/docker/build.sh +++ b/docker/build.sh @@ -11,12 +11,18 @@ set -e cd "$(dirname "$0")/.." +# Add -j 10 by default if no -j flag is given +ARGS=("$@") +if [[ ! " ${ARGS[*]} " =~ " -j " ]]; then + ARGS+=("-j" "10") +fi + # Start container if not running docker compose -f docker/docker-compose.yml up -d # Run build command (without asyncify - handled on host due to memory requirements) docker compose -f docker/docker-compose.yml exec kicad-wasm-builder \ - /workspace/scripts/kicad/build-pcbnew.sh "$@" + /workspace/scripts/kicad/build-pcbnew.sh "${ARGS[@]}" # Copy output to host-accessible directory echo "Copying build output to ./output/..." diff --git a/scripts/build-wasm-test.sh b/scripts/build-wasm-test.sh index 918b74e..47c6acf 100755 --- a/scripts/build-wasm-test.sh +++ b/scripts/build-wasm-test.sh @@ -3,15 +3,22 @@ # This script creates library symlinks and builds the test apps # # Usage: -# ./build-wasm-test.sh # Normal optimized build -# ./build-wasm-test.sh --debug # Debug build with DWARF symbols and source maps +# ./build-wasm-test.sh # Build all test apps (optimized) +# ./build-wasm-test.sh --debug # Build all test apps with debug symbols +# ./build-wasm-test.sh threadpool # Build only the threadpool test +# ./build-wasm-test.sh --debug menu # Build only menu test with debug symbols set -e DEBUG_BUILD=0 +TARGET="" + +# Parse arguments for arg in "$@"; do if [ "$arg" = "--debug" ]; then DEBUG_BUILD=1 + elif [ "$arg" != "" ]; then + TARGET="$arg" fi done @@ -28,6 +35,11 @@ if [ "$DEBUG_BUILD" = "1" ]; then else echo "Mode: Release (optimized)" fi +if [ -n "$TARGET" ]; then + echo "Target: $TARGET" +else + echo "Target: all" +fi echo "Project root: $PROJECT_ROOT" echo "wxWidgets build: $BUILD_DIR" echo "Test app dir: $WASM_APP_DIR" @@ -58,33 +70,61 @@ echo "" echo "=== Building test applications ===" cd "$WASM_APP_DIR" -# Clean any previous build -make -f Makefile.wasm clean 2>/dev/null || true - -# Build all (pass DEBUG flag if requested) -if [ "$DEBUG_BUILD" = "1" ]; then - # Debug build: -g for DWARF, -gsource-map for source maps, -O0 for no optimization - make -f Makefile.wasm DEBUG=1 +# Determine make target and clean behavior +if [ -n "$TARGET" ]; then + # Single target: only clean and build that specific target + MAKE_TARGET="$TARGET" + # Clean just the target's files + rm -f "standalone/$TARGET/${TARGET}_test.o" \ + "standalone/$TARGET/${TARGET}_test.html" \ + "standalone/$TARGET/${TARGET}_test.js" \ + "standalone/$TARGET/${TARGET}_test.wasm" \ + 2>/dev/null || true else - make -f Makefile.wasm + # All targets: clean everything first + MAKE_TARGET="all" + make -f Makefile.wasm clean 2>/dev/null || true +fi + +# Build (pass DEBUG flag if requested) +if [ "$DEBUG_BUILD" = "1" ]; then + make -f Makefile.wasm DEBUG=1 "$MAKE_TARGET" +else + make -f Makefile.wasm "$MAKE_TARGET" fi echo "" echo "=== Build complete ===" -echo "" -echo "Main test app:" -ls -lh "$WASM_APP_DIR"/minimal_test.html 2>/dev/null || echo " (not built)" -echo "" -echo "Standalone test apps:" -for test in menu clipboard filedialog layout aui toolbar grid dialog timer tree; do - if [ -f "$STANDALONE_DIR/$test/${test}_test.html" ]; then - echo " $test: OK" - ls -lh "$STANDALONE_DIR/$test/${test}_test.html" +if [ -n "$TARGET" ]; then + # Show just the built target + echo "" + if [ -f "$STANDALONE_DIR/$TARGET/${TARGET}_test.html" ]; then + echo "Built: $TARGET" + ls -lh "$STANDALONE_DIR/$TARGET/${TARGET}_test.html" + elif [ -f "$WASM_APP_DIR/${TARGET}_test.html" ]; then + echo "Built: $TARGET" + ls -lh "$WASM_APP_DIR/${TARGET}_test.html" else - echo " $test: (not built)" + echo "Warning: Expected output file not found" fi -done +else + # Show all built targets + echo "" + echo "Main test app:" + ls -lh "$WASM_APP_DIR"/minimal_test.html 2>/dev/null || echo " (not built)" + + echo "" + echo "Standalone test apps:" + for test in menu clipboard filedialog layout aui toolbar grid dialog timer tree; do + if [ -f "$STANDALONE_DIR/$test/${test}_test.html" ]; then + echo " $test: OK" + ls -lh "$STANDALONE_DIR/$test/${test}_test.html" + else + echo " $test: (not built)" + fi + done +fi echo "" echo "To run the tests:" diff --git a/scripts/kicad/build-pcbnew.sh b/scripts/kicad/build-pcbnew.sh index ff94c07..9dbca15 100755 --- a/scripts/kicad/build-pcbnew.sh +++ b/scripts/kicad/build-pcbnew.sh @@ -199,7 +199,7 @@ emcmake cmake "${KICAD_DIR}" \ -DCMAKE_POLICY_VERSION_MINIMUM=3.5 \ -DCMAKE_CXX_FLAGS="${EXTRA_FLAGS} -pthread -sUSE_ZLIB=1 -DKICAD_USE_PLATFORM_WASM=1 -I${SYSROOT}/include" \ -DCMAKE_C_FLAGS="${EXTRA_FLAGS} -pthread -sUSE_ZLIB=1 -I${SYSROOT}/include" \ - -DCMAKE_EXE_LINKER_FLAGS="${LINKER_DEBUG_FLAGS} -pthread -sUSE_ZLIB=1 -sASYNCIFY=1 -sASYNCIFY_STACK_SIZE=65536 -sUSE_PTHREADS=1 -sPTHREAD_POOL_SIZE=8 -sPTHREAD_POOL_SIZE_STRICT=0 -sALLOW_MEMORY_GROWTH=1 -sINITIAL_MEMORY=256MB -sMAXIMUM_MEMORY=4GB -sEXPORTED_RUNTIME_METHODS=['ccall','cwrap','UTF8ToString','stringToUTF8','lengthBytesUTF8'] -L${SYSROOT}/lib ${STUBS_BUILD}/libgit2_stub.a ${STUBS_BUILD}/libcurl_stub.a" \ + -DCMAKE_EXE_LINKER_FLAGS="${LINKER_DEBUG_FLAGS} -pthread -sUSE_ZLIB=1 -sASYNCIFY=1 -sASYNCIFY_STACK_SIZE=65536 -sUSE_PTHREADS=1 -sPTHREAD_POOL_SIZE='navigator.hardwareConcurrency' -sPTHREAD_POOL_SIZE_STRICT=0 -sALLOW_MEMORY_GROWTH=1 -sINITIAL_MEMORY=256MB -sMAXIMUM_MEMORY=4GB -sEXPORTED_RUNTIME_METHODS=['ccall','cwrap','UTF8ToString','stringToUTF8','lengthBytesUTF8'] -L${SYSROOT}/lib ${STUBS_BUILD}/libgit2_stub.a ${STUBS_BUILD}/libcurl_stub.a" \ -DCMAKE_PREFIX_PATH="${SYSROOT};${WX_BUILD}" \ -DwxWidgets_CONFIG_EXECUTABLE="${WX_BUILD}/wx-config" \ \ diff --git a/tests/e2e/threadpool.spec.ts b/tests/e2e/threadpool.spec.ts new file mode 100644 index 0000000..b42375d --- /dev/null +++ b/tests/e2e/threadpool.spec.ts @@ -0,0 +1,89 @@ +import { test, expect, tryLoadApp } from './utils/fixtures'; + +test.describe('Thread Pool Deadlock Tests', () => { + + test('Creating hardware_concurrency threads should not deadlock', async ({ page, testLogger }) => { + // This test replicates KiCad's BS::priority_thread_pool pattern: + // - Creates hardware_concurrency() threads in the frame constructor + // - If PTHREAD_POOL_SIZE < hardware_concurrency, this will deadlock because: + // 1. Threads 1-N use pre-warmed Web Workers + // 2. Thread N+1 needs new Web Worker (posts to event loop) + // 3. Main thread busy-waits for thread to start + // 4. Busy-wait blocks event loop -> Worker message never processed + // 5. DEADLOCK (timeout) + + await page.goto('/standalone/threadpool/threadpool_test.html'); + + // If there's a deadlock, tryLoadApp will timeout waiting for canvas + const loaded = await tryLoadApp(page, 30000); + + await page.screenshot({ path: 'test-results/threadpool-01-loaded.png', fullPage: true }); + + // Check console for hardware_concurrency value + const hardwareConcurrencyLog = testLogger.consoleLogs.find(log => + log.includes('[THREADPOOL] hardware_concurrency:') + ); + if (hardwareConcurrencyLog) { + console.log('Detected:', hardwareConcurrencyLog); + } + + // Check for success marker - all threads created and joined + const success = testLogger.consoleLogs.some(log => + log.includes('[THREADPOOL] SUCCESS') + ); + + // Check for blocking warning (indicates potential deadlock situation) + const blockingWarning = testLogger.consoleLogs.some(log => + log.includes('Blocking on the main thread is very dangerous') + ); + + expect(loaded, 'App should load without deadlock').toBe(true); + expect(success, 'All threads should complete successfully').toBe(true); + + // Log blocking warning status (informational, doesn't fail test) + if (blockingWarning) { + console.log('Warning: Blocking on main thread detected (thread creation may have triggered worker spawn)'); + } + + expect(testLogger.errors.filter(e => !e.includes('favicon'))).toHaveLength(0); + }); + + test('Thread creation logs show all threads started', async ({ page, testLogger }) => { + await page.goto('/standalone/threadpool/threadpool_test.html'); + + const loaded = await tryLoadApp(page, 30000); + if (!loaded) { + test.skip(); + return; + } + + // Extract hardware_concurrency from logs + const hwLog = testLogger.consoleLogs.find(log => + log.includes('[THREADPOOL] hardware_concurrency:') + ); + + if (!hwLog) { + console.log('Could not find hardware_concurrency log'); + return; + } + + const match = hwLog.match(/hardware_concurrency:\s*(\d+)/); + if (!match) { + console.log('Could not parse hardware_concurrency value'); + return; + } + + const expectedThreads = parseInt(match[1], 10); + console.log(`Expected ${expectedThreads} threads based on hardware_concurrency`); + + // Count "Thread X started" messages + const threadStartedLogs = testLogger.consoleLogs.filter(log => + log.includes('[THREADPOOL] Thread') && log.includes('started') + ); + + console.log(`Found ${threadStartedLogs.length} "Thread started" messages`); + + // All threads should have started + expect(threadStartedLogs.length).toBe(expectedThreads); + }); +}); diff --git a/tests/wasm-app/Makefile.wasm b/tests/wasm-app/Makefile.wasm index 4e53da2..888fdd5 100644 --- a/tests/wasm-app/Makefile.wasm +++ b/tests/wasm-app/Makefile.wasm @@ -86,6 +86,13 @@ LDFLAGS_ADV = $(DEBUG_LDFLAGS) $(BASE_LDFLAGS) $(WX_LDFLAGS_ADV) # LDFLAGS for XML apps (xml_test) LDFLAGS_XML = $(DEBUG_LDFLAGS) $(BASE_LDFLAGS) $(WX_LDFLAGS_XML) +# LDFLAGS for pthread test - uses dynamic PTHREAD_POOL_SIZE to match hardware_concurrency +# This fixes the KiCad deadlock: pre-warm enough workers for all threads +LDFLAGS_PTHREAD = $(DEBUG_LDFLAGS) $(BASE_LDFLAGS) -pthread \ + -sPTHREAD_POOL_SIZE='navigator.hardwareConcurrency' \ + -sPTHREAD_POOL_SIZE_STRICT=0 \ + $(WX_LDFLAGS_NOGL) + JS = $(TOOLS_ROOT)/wx.js HTML = $(TOOLS_ROOT)/template.html @@ -132,7 +139,8 @@ all: minimal_test.html \ $(S)/bitmask/bitmask_test.html \ $(S)/regions/regions_test.html \ $(S)/maximize/maximize_test.html \ - $(S)/earlysize/earlysize_test.html + $(S)/earlysize/earlysize_test.html \ + $(S)/threadpool/threadpool_test.html # Main test app (uses GL) minimal_test.o: minimal_test.cpp @@ -421,6 +429,13 @@ $(S)/earlysize/earlysize_test.o: $(S)/earlysize/earlysize_test.cpp $(S)/earlysize/earlysize_test.html: $(S)/earlysize/earlysize_test.o $(CXX) $< $(LDFLAGS_NOGL) --pre-js $(JS) --shell-file $(HTML) -o $@ +# Thread Pool test (pthread) - reproduces KiCad deadlock when hardware_concurrency() > PTHREAD_POOL_SIZE +$(S)/threadpool/threadpool_test.o: $(S)/threadpool/threadpool_test.cpp + $(CXX) -c $(CXXFLAGS) -pthread $< -o $@ + +$(S)/threadpool/threadpool_test.html: $(S)/threadpool/threadpool_test.o + $(CXX) $< $(LDFLAGS_PTHREAD) --pre-js $(JS) --shell-file $(HTML) -o $@ + # Convenience targets menu: $(S)/menu/menu_test.html clipboard: $(S)/clipboard/clipboard_test.html @@ -462,9 +477,10 @@ bitmask: $(S)/bitmask/bitmask_test.html regions: $(S)/regions/regions_test.html maximize: $(S)/maximize/maximize_test.html earlysize: $(S)/earlysize/earlysize_test.html +threadpool: $(S)/threadpool/threadpool_test.html clean: rm -f minimal_test.o minimal_test.html minimal_test.js minimal_test.wasm rm -f $(S)/*/*.o $(S)/*/*.html $(S)/*/*.js $(S)/*/*.wasm -.PHONY: all clean menu clipboard filedialog layout aui toolbar grid dialog timer tree dataview htmlwin stc print dnd propgrid pickers collapsible listctrl infobar dataviewvirtual auinotebook wizard gridedit calendar gridrenderers printpreview bitmapbuttons specialized validators ownerdrawn popup xml wasmedge fontenum textdecor bitmask regions maximize earlysize +.PHONY: all clean menu clipboard filedialog layout aui toolbar grid dialog timer tree dataview htmlwin stc print dnd propgrid pickers collapsible listctrl infobar dataviewvirtual auinotebook wizard gridedit calendar gridrenderers printpreview bitmapbuttons specialized validators ownerdrawn popup xml wasmedge fontenum textdecor bitmask regions maximize earlysize threadpool diff --git a/tests/wasm-app/standalone/threadpool/threadpool_test.cpp b/tests/wasm-app/standalone/threadpool/threadpool_test.cpp new file mode 100644 index 0000000..24b0210 --- /dev/null +++ b/tests/wasm-app/standalone/threadpool/threadpool_test.cpp @@ -0,0 +1,146 @@ +/** + * Thread Pool Deadlock Test + * + * This test replicates KiCad's pattern of creating hardware_concurrency() threads. + * When PTHREAD_POOL_SIZE < hardware_concurrency(), this should deadlock because: + * 1. Threads 1-N use pre-warmed Web Workers + * 2. Thread N+1 needs new Web Worker (posts to event loop) + * 3. Main thread busy-waits for thread to start + * 4. Busy-wait blocks event loop -> Worker message never processed + * 5. DEADLOCK + */ + +#include "wx/wx.h" +#include +#include +#include +#include + +#ifdef __EMSCRIPTEN__ +#include +#endif + +class ThreadPoolFrame : public wxFrame +{ +public: + ThreadPoolFrame() + : wxFrame(nullptr, wxID_ANY, "Thread Pool Deadlock Test", + wxDefaultPosition, wxSize(800, 600)) + { + // Log hardware_concurrency - this is what KiCad uses to determine thread count + int num_threads = std::thread::hardware_concurrency(); + +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] hardware_concurrency: ' + $0); + }, num_threads); +#endif + printf("[THREADPOOL] hardware_concurrency: %d\n", num_threads); + + // Replicate KiCad's exact pattern: create hardware_concurrency() threads + // in the constructor (like BS::priority_thread_pool does) + printf("[THREADPOOL] Creating %d threads (like KiCad's BS::priority_thread_pool)...\n", num_threads); + +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] Creating ' + $0 + ' threads...'); + }, num_threads); +#endif + + std::vector threads; + std::vector> started(num_threads); + + for (int i = 0; i < num_threads; i++) { + started[i] = false; + + printf("[THREADPOOL] Creating thread %d\n", i); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] Creating thread ' + $0); + }, i); +#endif + + threads.emplace_back([i, &started]() { + started[i] = true; +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] Thread ' + $0 + ' started'); + }, i); +#endif + // Simulate some work + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + }); + } + + printf("[THREADPOOL] All threads created, waiting for completion...\n"); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] All threads created, waiting for completion...'); + }); +#endif + + // Join all threads (this is what thread pool destructor does) + for (auto& t : threads) { + t.join(); + } + + printf("[THREADPOOL] SUCCESS - All %d threads completed!\n", num_threads); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] SUCCESS - All threads completed!'); + }); +#endif + + // Create simple UI to show success + wxPanel* panel = new wxPanel(this); + wxBoxSizer* sizer = new wxBoxSizer(wxVERTICAL); + + wxString msg = wxString::Format( + "Thread Pool Test PASSED!\n\n" + "Created and joined %d threads successfully.\n\n" + "This test replicates KiCad's BS::priority_thread_pool pattern.\n" + "If you see this message, the deadlock did NOT occur.", + num_threads + ); + + wxStaticText* label = new wxStaticText(panel, wxID_ANY, msg, + wxDefaultPosition, wxDefaultSize, wxALIGN_CENTER); + label->SetFont(wxFont(14, wxFONTFAMILY_DEFAULT, wxFONTSTYLE_NORMAL, wxFONTWEIGHT_NORMAL)); + + sizer->AddStretchSpacer(); + sizer->Add(label, 0, wxALIGN_CENTER | wxALL, 20); + sizer->AddStretchSpacer(); + + panel->SetSizer(sizer); + CreateStatusBar(); + SetStatusText(wxString::Format("SUCCESS: %d threads created and joined", num_threads)); + } +}; + +class ThreadPoolApp : public wxApp +{ +public: + virtual bool OnInit() override + { + printf("[THREADPOOL] App OnInit starting\n"); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] App OnInit starting'); + }); +#endif + + ThreadPoolFrame* frame = new ThreadPoolFrame(); + frame->Show(); + + printf("[THREADPOOL] App OnInit complete\n"); +#ifdef __EMSCRIPTEN__ + EM_ASM({ + console.log('[THREADPOOL] App OnInit complete'); + }); +#endif + + return true; + } +}; + +wxIMPLEMENT_APP(ThreadPoolApp);