OBS mis-pairs batch results under Parallel execution (upstream bug)

[Agash]

Under RequestBatchExecutionType.Parallel, OBS pairs each result's requestType/requestId with a different request's responseData/requestStatus. No client can correct this: the association is already lost when the response is built. Serial execution is unaffected.

This issue is for tracking on our side. The fix belongs in obs-websocket; the analysis below is written so it can be handed over as-is.

Root cause

src/requesthandler/RequestBatchHandler.cpp collects parallel results in completion order. Each thread-pool task appends as it finishes:

for (auto &request : requests) {
    threadPool.start(Utils::Compat::CreateFunctionRunnable([&parallelResults, &request]() {
        RequestResult requestResult = parallelResults.requestHandler.ProcessRequest(request);

        std::unique_lock<std::mutex> lock(parallelResults.conditionMutex);
        parallelResults.results.push_back(requestResult);   // completion order
        lock.unlock();
        parallelResults.condition.notify_one();
    }));
}

src/websocketserver/WebSocketServer_Protocol.cpp then zips that vector against the original submission-order request array, by index:

size_t i = 0;
for (auto &requestResult : resultsVector) {
    results.push_back(ConstructRequestResult(requestResult, requests[i]));
    i++;
}

ConstructRequestResult takes the identity from one and the outcome from the other:

ret["requestType"]   = requestJson["requestType"];      // submission order
ret["requestId"]     = requestJson["requestId"];        // submission order
ret["requestStatus"] = { ... requestResult ... };       // completion order
ret["responseData"]  = requestResult.ResponseData;      // completion order

While the two orders differ, every result is mislabelled. The serial paths build their vector in submission order, so they line up.

Note requestStatus is displaced along with responseData, so a parallel batch can report a failure against a request that actually succeeded.

Observed

Raw bytes off the wire for a parallel batch of GetVersion, GetSceneItemList, GetStats:

"requestType":"GetVersion",       "responseData":{"sceneItems"...
"requestType":"GetSceneItemList", "responseData":{"activeFps"...
"requestType":"GetStats",         "responseData":{"availableRequests"...

Every serial batch in the same run pairs correctly.

Suggested fix

Preserve submission order in the parallel branch: size the results vector up front and have each task write to its own slot, rather than appending. The existing requests[i] zip in WebSocketServer_Protocol.cpp then needs no change, and requestStatus is fixed at the same time.

parallelResults.results.resize(requests.size());

for (size_t index = 0; index < requests.size(); index++) {
    auto &request = requests[index];
    threadPool.start(Utils::Compat::CreateFunctionRunnable([&parallelResults, &request, index]() {
        RequestResult requestResult = parallelResults.requestHandler.ProcessRequest(request);

        std::unique_lock<std::mutex> lock(parallelResults.conditionMutex);
        parallelResults.results[index] = requestResult;   // submission order
        parallelResults.completed++;
        lock.unlock();
        parallelResults.condition.notify_one();
    }));
}

The wait condition currently uses results.size() == requestCount, which no longer works once the vector is pre-sized, so it needs a separate completion counter.

Why this cannot be worked around client-side

• Nothing in the response reveals the completion order, so the permutation is not recoverable.
• inputVariables/outputVariables would allow a marker, but are rejected in parallel mode.
• responseData is built by each request handler and has no passthrough field to stamp an id into.
• Matching payloads by shape fails: 75 of 147 requests return no payload at all, and five groups of requests share identical response shapes (seven of them return just outputActive). It also cannot recover requestStatus.

Client behaviour meanwhile

BatchResults.Get and TryGet refuse to resolve a reference on a parallel batch and explain why, rather than returning another request's data. Raw remains available.

[Agash]

The mispairing is worse on MessagePack than on JSON

Measured while documenting what a caller can still recover. Only the labelling is displaced, so requestStatus and responseData do belong together on a row and every payload is present. Reading them back is where the transports part company.

A parallel batch of GetVersion, GetVideoSettings, GetStats, then each row read as each of the three response records:

JSON rejects the wrong type, so probing works:

row 0: as GetVersionResponseData -> 32.2.2
       as GetVideoSettingsResponseData -> threw ObsWebSocketSerializationException
       as GetStatsResponseData -> threw ObsWebSocketSerializationException

MessagePack accepts every one of them:

row 0: as GetVersionResponseData -> (empty)
       as GetVideoSettingsResponseData -> 30/1 1920x1080
       as GetStatsResponseData -> cpu=0.00 mem=0.0

MessagePack maps by key name here, so a payload read as the wrong record leaves the unmatched properties at their defaults and returns an object rather than failing. cpu=0.00, mem=0.0 is indistinguishable from a real reading.

Two consequences for us. The refusal in BatchResults.Get is doing more work than it looked like: on MessagePack there is no downstream error that would catch a mispaired read. And it rules out the shape-matching recovery idea for good, since on the transport where recovery would matter most, shapes do not fail to match.

Documented in the README under Running requests in parallel: a homogeneous parallel batch is recoverable through Raw, a heterogeneous one is not.