Skip to content

Race conditions and stability improvements from Lawless Legends fork #60

Description

@badvision

Overview

While working on the Lawless Legends fork, we identified and fixed several race conditions and stability issues in the core emulator that would benefit the base Jace project. These fixes address threading issues, state management problems, and deadlock scenarios during reboot operations.

Reference commit in lawless-legends fork:
badvision/lawless-legends@e2d6af7

Relevant Fixes for Base Jace

1. Worker Thread Synchronization Race Condition

File: src/main/java/jace/core/IndependentTimedDevice.java

Problem: isRunning() returns true before worker thread actually starts executing, creating timing windows where code assumes the device is running but the worker thread hasn't entered its event loop yet.

Fix: Added CountDownLatch in resume() to ensure worker thread signals when it has actually started:

@Override
public synchronized void resume() {
    super.resume();
    if (worker != null && worker.isAlive()) {
        return;
    }
    
    CountDownLatch workerStarted = new CountDownLatch(1);
    Thread newWorker = new Thread(() -> {
        workerStarted.countDown();  // Signal started FIRST
        while (isRunning()) {
            // ... event loop
        }
    });
    this.worker = newWorker;
    newWorker.start();
    
    try {
        if (!workerStarted.await(1000, TimeUnit.MILLISECONDS)) {
            LOGGER.warning("Worker thread did not start within 1 second");
        }
    } catch (InterruptedException e) {
        Thread.currentThread().interrupt();
    }
}

Impact: Prevents race conditions in any code that waits for device operations after calling resume().

2. VBL Semaphore Deadlock Prevention

File: jace/lawless/LawlessComputer.java (relevant pattern for any VBL waiting code)

Problem: Code that waits for VBL signals can deadlock if:

  • Worker thread check passes but thread dies before VBL generated
  • isRunning() returns true but worker not actually running

Fix: Defensive checks with timeout:

public void waitForVBL(int count) throws InterruptedException {
    Video video = getVideo();
    Motherboard mb = getMotherboard();
    if (video == null || mb == null) return;
    if (!video.isRunning() || !mb.isRunning()) return;
    
    // Check worker thread actually alive
    if (mb instanceof IndependentTimedDevice) {
        Thread worker = ((IndependentTimedDevice) mb).getWorkerThread();
        if (worker == null || !worker.isAlive()) {
            LOGGER.warning("Motherboard worker thread not running, skipping VBL wait");
            return;
        }
    }
    
    Semaphore s = new Semaphore(0);
    onNextVBL(s::release);
    if (!s.tryAcquire(2000, TimeUnit.MILLISECONDS)) {
        vblCallbacks.removeIf(r -> r == s::release);
        LOGGER.warning("VBL wait timed out after 2 seconds");
        return;
    }
}

Impact: Prevents infinite hangs when waiting for video signals during lifecycle transitions.

3. Optional NPE Prevention in Headless Mode

File: src/main/java/jace/hardware/DiskIIDrive.java

Problem: Calling Optional.get() without checking causes NoSuchElementException crashes in headless mode.

Fix: Use ifPresent() pattern:

public void addIndicator() {
    long now = System.currentTimeMillis();
    if (lastAdded == 0 || now - lastAdded >= 500) {
        icon.ifPresent(i -> {  // Changed from unsafe .get()
            EmulatorUILogic.addIndicator(this, i);
            lastAdded = now;
        });
    }
}

public void removeIndicator() {
    icon.ifPresent(i -> EmulatorUILogic.removeIndicator(this, i));
}

Impact: Allows disk drive operations to work correctly in headless/testing scenarios.

4. Memory Configuration Cache Invalidation

File: src/main/java/jace/apple2e/RAM128k.java

Problem: Memory configuration cache not cleared during state resets, causing stale card ROM mappings.

Fix: Clear caches in resetState():

@Override
public void resetState() {
    state = "???";
    memoryConfigurations.clear();  // Force rebuild
    banks = null;  // Force refresh
}

Impact: Ensures memory configuration is rebuilt from current state, not stale cache.

5. Complete State Reset During coldStart

File: Pattern applicable to Computer.coldStart() implementations

Problem: Partial state cleanup with race windows where workers resume before complete reinitialization, causing multiple failure modes (BASIC prompt, self-test mode, freeze).

Fix: Perform ALL cleanup and initialization inside whileSuspended() before workers resume:

@Override
public void coldStart() {
    getMotherboard().whileSuspended(()->{
        // PHASE 1: COMPLETE STATE CLEANUP
        RAM128k ram = (RAM128k) getMemory();
        ram.zeroAllRam();
        ram.resetState();  // Clear caches
        blankTextPage1();
        for (SoftSwitches s : SoftSwitches.values()) {
            s.getSwitch().reset();
        }
        
        // PHASE 2: COMPLETE REINITIALIZATION (before resume)
        getMemory().configureActiveMemory();
        getVideo().configureVideoMode();
        getCpu().reset();
        for (Optional<Card> c : getMemory().getAllCards()) {
            c.ifPresent(Card::reset);
        }
    });
    // Workers resume here with complete state
}

Impact: Eliminates race conditions between state reset and worker thread resumption.

Testing

Created comprehensive stress test suite demonstrating these issues:

  • RebootStabilityStressTest.java - 4 test scenarios, 50+ iterations each
  • Tests cover upgrade timing races, concurrent operations, state reset verification, and full reboot cycles

Recommendation

These fixes address fundamental threading and lifecycle issues in the core emulator. While discovered during Lawless Legends development, they apply to any Jace-based project and would improve stability for:

  • Automated testing scenarios
  • Headless operation
  • Reboot/reset operations
  • Any code using VBL synchronization
  • Multi-threaded emulator control

Non-Applicable Changes

The commit also includes Lawless Legends-specific changes (upgrade handling, boot watchdog, etc.) that are not relevant to base Jace, but the threading and lifecycle fixes above are universally applicable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions