Skip to content

Add async script-file jobs - #36

Merged
ghirpara merged 4 commits into
bluemoonfoundry:masterfrom
abrady:feature/async-script-file
Aug 20, 2026
Merged

Add async script-file jobs#36
ghirpara merged 4 commits into
bluemoonfoundry:masterfrom
abrady:feature/async-script-file

Conversation

@abrady

@abrady abrady commented Aug 19, 2026

Copy link
Copy Markdown

Summary

  • accept scriptFile on POST /execute/async with the existing file-over-inline precedence
  • retain the script path through the async queue and load it on Daz's main thread
  • preserve getScriptFileName() under the Studio 6 evaluate() result-capture wrapper
  • let async execute and registered-script submissions enqueue while Daz's main thread is busy
  • expose file-job submission from both synchronous and asynchronous dazpy clients

Architecture

Previously, a busy Daz main thread could block or reject an async submission before it entered the queue. Now the HTTP worker enqueues it immediately for later execution, so submitted work is retained and can be monitored or cancelled while Daz finishes the current job.

HTTP worker threads only parse and validate request value data, then submit it to the mutex-protected AsyncRequestManager. The manager owns queue and lifecycle state. DazScript loading and execution remain on Daz's main thread in processNextAsyncRequest().

This keeps submission genuinely asynchronous without moving Daz or UI work off the main thread. UI logging from worker paths is posted back through the existing queued appendLogBytes slot.

Verification

  • 722 Python tests passed, plus 30 subtests
  • Studio 6 Release plugin built against SDK 6.25 / Qt 6.10.3 with 0 errors
  • prior live acceptance verified exact file identity, prompt async submission while Daz was busy, and cancellation of a queued file job

Scope

This PR is intentionally limited to async script-file execution and queue responsiveness. Structured job observation, broader dazpy protocol changes, and the shared scene/render/runtime work have been removed for focused follow-up PRs.

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Live acceptance completed on Daz Studio 6.25.2026.14722 / Qt 6.10.3.

Verified:

  • compatible Studio 6 Release plugin builds and loads
  • sync and async scriptFile both preserve the exact getScriptFileName() value
  • an async submit returns in ~2 ms while Daz's main thread is occupied
  • a second queued file job can be cancelled before execution
  • 698 sync unit tests pass
  • 24 async-client tests pass
  • all 76 live HTTP API tests pass

The first worker-thread version exposed a real Qt affinity crash in appendLog() (QTextEdit::append from AsyncScriptHandler). The supplied Daz crash report pinned that stack; logging now uses the existing queued appendLogBytes path. The complete live suite passes after the correction and Daz remains healthy.

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 1B is now included in e3e2f7f: async script jobs can name a per-job JSONL report file, which the plug-in incrementally ingests into structured progress, a bounded 100-entry log tail with truncation metadata, and a deduplicated output manifest. The report path is injected as getArguments()[0].__dssReportFile. Validation: compatible DS6 Release build; 700 dazpy tests + 30 subtests; all 78 live HTTP API tests; live progress remained queryable while Daz's main thread was busy. The installed DS6 plug-in is running healthy.

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 2 is now in dc9fd6d: dazpy owns registered-script registration/execution, detailed request and render cancellation, long-poll timeout policy, typed generic server errors, and SSE error mapping. Validation: 706 tests plus 30 subtests; compileall clean; live MCP adapter exercised status, registered scene-info, and request listing against Daz Studio.

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 6 shared-runtime update: a904671 adds verified four-channel visibility shared by queued DazScript recipes and dazpy; cb8aa16 adds structured progress/log/output reporting shared by recipes and async observation. Live validation against Daz Studio 6.25: 706 server/dazpy unit tests, 27 async-client tests, 81 live HTTP API tests, plus focused runtime contract tests. Afterglow delegates both invariants while retaining Script IDE fallbacks (76a02e3, f5fa30e).

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 6 node-resolution slice landed in 95c4a65. DSS.nodes now owns exact label/name/either lookup, missing-node diagnostics with nearby candidates, and duplicate rejection; dazpy discovery and generated node locators delegate to it. Validation: 707 server/dazpy tests, 27 async-client tests, 82 live HTTP tests. Afterglow delegation is pushed as 89267c7 (rebased over concurrent main).

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 6 render-settings slice landed in dfa9614. DSS.render now applies and verifies output path, paired dimensions, direct-to-file/software mode, viewport override, camera, and Iray sample/time caps. dazpy setters/render and Afterglow materializer/capsule delegate to it. Validation: 709 unit tests, 27 async-client tests, 83 live HTTP tests. Afterglow delegation is pushed as 54bf3c7 over concurrent main.

@abrady

abrady commented Aug 19, 2026

Copy link
Copy Markdown
Author

Stage 6 gained two more shared-runtime slices:

  • Scene/file identity: fe078d0 adds DSS.scene.identity/require/load with explicit merge-vs-replace intent, source existence checks, minimum-node assertions, normalized filename readback, and dazpy delegation.
  • Simulation setup: 094f324 adds DSS.simulation.state/configure/clear/run, synchronizes animation + playback ranges with readback, keeps real runs on async jobs, and refuses empty-scene clear after live testing showed Daz can block for over a minute initializing dForce with nothing to clear.

Current gates: 713 unit, 27 async-client, 85 live API. The compatible DS6 plugin was rebuilt, installed, restarted, and the focused runtime contract passed live.

@ghirpara

Copy link
Copy Markdown
Member

Hi @abrady It looks like you've added a number of changes onto the PR that seem to be rather large for the narrow issue that you identified.

Can you check your code and limit the submission to the specific problem of the async execute not picking up the scriptFile (and maybe the async-queue responsiveness fix) but leave out the broad scene, render, and runtime refactors unless they're directly related to this problem?

I'm fine with incorporating the other fixes, but they should be in a separate PR(s).

@abrady abrady changed the title Add async script-file jobs Unify observable async Daz automation Aug 19, 2026
@abrady
abrady force-pushed the feature/async-script-file branch from 9eecdf5 to b4f746d Compare August 19, 2026 20:46
@abrady abrady changed the title Unify observable async Daz automation Add async script-file jobs Aug 19, 2026
@ghirpara

ghirpara commented Aug 20, 2026

Copy link
Copy Markdown
Member

@abrady

I ran Claude /code-review against the current diff (origin/master...pr-36). Four findings; the first two are blockers given the repo's documented threading model. Digging into the results they do seem to make sense. Can you validate and patch?

🚫 Blocker: src/RequestHandlers.cpp:241,260 — Qt work now runs on httplib worker threads

AsyncExecuteHandler::handle / AsyncScriptHandler::handle now call handleAsyncExecuteEnqueue / handleAsyncScriptEnqueue directly instead of routing through Qt::BlockingQueuedConnection. Internally these construct QFileInfo and do filesystem stats (via RequestValidator::validateExecuteFields) — Qt operations executing off the main thread, which CLAUDE.md's threading model explicitly prohibits:

"Handlers must do minimal work (parse body), then invoke handleExecuteRequest() on main thread via Qt::BlockingQueuedConnection. All QScriptEngine, DzScript, and Qt operations MUST happen on the main thread."

Even if the specific fields touched here are safe in practice, this is a direct violation of the stated invariant the rest of the codebase relies on.

🚫 Blocker: src/DzScriptServerPane.cpp:1776 — unsynchronized cross-thread read of m_nMaxScriptLengthKB

As a consequence of the above, m_nMaxScriptLengthKB is now read from HTTP worker threads via validateExecuteFields, while it's written from the GUI thread when a user changes the "Max Script Length" setting (DzScriptServerPane.cpp:532). This is a data race on a plain int with no atomic/mutex protection — undefined behavior, and could read a torn/inconsistent limit. Previously this path only ran on the main thread, so no race existed.

Non-blocking: src/DzScriptServerPane.cpp:2944 — possible cross-request filename leak

The reused persistent DzScript instance may not clear its retained filename between jobs. A scriptFile-backed async job followed immediately by an inline-script async job on the same reused instance could report the previous job's file path from getScriptFileName() instead of empty. This was invisible before this PR (since getScriptFileName() always returned empty under evaluate()) but is now user-visible. No test exercises this file→inline sequence — worth adding one.

Non-blocking: src/DzScriptServerPane.cpp:1775 — missing WARN log parity

handleAsyncExecuteEnqueue silently prioritizes scriptFile over script when both are provided, but unlike the sync handler (handleExecuteRequest, lines 1535-1539) it never logs the "Both scriptFile and script provided" WARN — losing debuggability parity with the sync path.


Findings 1 and 2 should be resolved before merge — either by keeping enqueue's Qt-touching validation on the main thread, or by explicitly documenting/justifying the departure from the threading model and adding proper synchronization for the shared config read.

@abrady
abrady marked this pull request as ready for review August 20, 2026 03:06
@ghirpara

Copy link
Copy Markdown
Member

Follow-up review — commit a14f9fa

Great progress on the first two blockers:

  • Qt-off-main-thread: the threading model doc was corrected (not just relaxed) to distinguish QObject/DAZ-SDK work (main-thread only) from locally-constructed reentrant Qt Core value types, which is consistent with Qt's own threading guarantees. Looks legitimate rather than a rule change to dodge the finding.
  • m_nMaxScriptLengthKB race: fixed properly — the limit is now snapshotted into AsyncExecuteHandler's constructor before httplib starts its worker threads and passed explicitly through handleAsyncExecuteEnqueue(jsonBody, clientIP, maxScriptLengthKB). No more live cross-thread read of GUI-owned state.
  • Filename leak (non-blocking finding [Refactoring] Phase 1: Testing Infrastructure and Critical Thread Safety #3): also fixed — runDazScript now takes an explicit scriptFilename and injects a getScriptFileName() shim directly, rather than depending on the reused DzScript instance's retained state.

🚫 New blocker introduced by this update: src/RequestHandlers.cpp — SDK4 async handlers lost their busy-thread guard

respondIfMainThreadBusy(m_pPane, ctx) was removed entirely from both AsyncExecuteHandler::handle and AsyncScriptHandler::handle. The #if DAZ_SDK_MAJOR_VERSION >= 6 branch correctly replaces the blocking crossing with a direct, worker-thread-safe call — but the #else (Studio 4 / SDK4) branch still does the same no-timeout Qt::BlockingQueuedConnection as before, now with no guard.

On an SDK4 build, if the main thread is busy running a script, any subsequent POST /execute/async or POST /scripts/:id/async request now blocks the httplib worker thread indefinitely instead of getting a 503. Enough concurrent submissions during a busy period exhaust httplib's fixed worker pool, taking down the whole server — including sync /execute, /status, and /health — which is exactly the failure mode respondIfMainThreadBusy exists to prevent (it's still correctly used by the sync/render handlers elsewhere in this file).

Fix: restore respondIfMainThreadBusy inside the #else (SDK4) branch, or guard both branches ahead of the #if.

No other issues found — the scriptFile plumbing through AsyncRequestManager, the getScriptFileName() shim, and the maxScriptLengthKB snapshot are all internally consistent with their documented rationale.

@ghirpara ghirpara self-assigned this Aug 20, 2026
@ghirpara
ghirpara merged commit 7377239 into bluemoonfoundry:master Aug 20, 2026
3 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.

4 participants