Skip to content

Connectors: fetch command type and payload in a single round-trip (getCommandInfo)#311

Open
biohazardxxx wants to merge 10 commits into
vdemydiuk:devfrom
biohazardxxx:feat/connector-merged-command-fetch
Open

Connectors: fetch command type and payload in a single round-trip (getCommandInfo)#311
biohazardxxx wants to merge 10 commits into
vdemydiuk:devfrom
biohazardxxx:feat/connector-merged-command-fetch

Conversation

@biohazardxxx

Copy link
Copy Markdown

Summary

Note: this branch is stacked on #308 (payload truncation fix) and includes its commits — the new work is the last 6 commits. Merge #308 first and this PR reduces to its own changes.

Fetching one command currently costs two blocking cross-thread round-trips into the connector's io thread: getCommandType (posts a packaged_task, blocks) then getPayload inside the handler (posts another, blocks). This PR merges them into one:

  • MtService: new GetCommandInfo(handle, int& type, std::string& payload) — a single packaged_task that pops the next command AND returns its payload. Existing methods untouched; everything touching expert state still runs on the single io thread.
  • Both connectors export:
    // pops the next command; returns required payload length in wchar_t (excl. null) or -1 on error;
    // copies at most capacity-1 chars (always null-terminated); type receives the command type
    int getCommandInfo(int expertHandle, int& type, wchar_t* payload, int capacity, wchar_t* err);
  • Both EAs (MQL5 + MQL4) fetch type and payload in one call into a module-level _command_payload; GetJsonPayload() parses that instead of calling the DLL — all Execute_* handlers untouched. Oversized payloads re-fetch via getPayload2 (safe: GetCommandPayload reads current_task_ non-destructively; the command is not re-popped).
  • getCommandType/getPayload/getPayload2 remain exported and functional — old .ex4/.ex5 binaries keep working with the new DLLs (live-verified, see Testing).

Implementation notes

  • Adversarial review found one medium defect: a payload conversion failure after the pop (e.g. invalid UTF-8) silently dropped the command, leaving the client waiting the full CommandTimeout where the old path returned an immediate error. Fixed in 364b193: the EA now dispatches with an empty payload so the handler emits the same immediate "Failed to get payload" error response as before.
  • Review also confirmed: single packaged_task (no nested post/deadlock), empty-queue loop exits identical to the old path, stdcall decoration correct on Win32 (_getCommandInfo@20), no stale-payload leakage across commands.

Testing

  • Both connectors build clean (x64 + Win32); getCommandInfo present in both DLLs' export tables (dumpbin). Both EAs compile Result: 0 errors, 0 warnings; recompiled .ex5/.ex4 included.
  • Runtime-verified against a live MT5 terminal (build 5830) with the new DLL + new EA:
    • Short and 6016-char Print (exercising the oversized re-fetch path): both return true.
    • 50 sequential commands: 0.5–0.6 ms/command end-to-end through the merged path.
    • Backward compat: two experts running the previous .ex5 on the same new DLL answered commands normally via the old exports.
  • MQL4 side is source-identical and compiles clean but was not runtime-tested against an MT4 terminal.

🤖 Generated with Claude Code

biohazardxxx and others added 10 commits July 6, 2026 12:33
convertSystemString copied at most 1000 wchars into the caller's MQL
string buffer regardless of its actual capacity and never wrote a null
terminator (stale content from a previous, longer error message could
leak into reused buffers). Command payloads longer than 1000 characters
arrived truncated in the EA and failed JSON parsing.

- convertSystemString now takes the destination capacity, bounds the
  copy and always null-terminates.
- getPayload keeps its signature but honors the EA's real 5000-wchar
  payload buffer (StringInit(payload, 5000, 0)) instead of 1000.
- New size-aware export getPayload2(handle, res, capacity, err) returns
  the full payload length (or -1 on error) and copies at most
  capacity-1 chars, so callers can re-allocate and fetch payloads of
  any size.

Applies to both MT5Connector and MTConnector.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GetJsonPayload now asks the connector for the payload with an explicit
buffer capacity and re-allocates when the reported size exceeds the
default 5000-character buffer, removing the previous hard cap on
inbound command payloads.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same change as the MQL5 expert: GetJsonPayload re-allocates its buffer
based on the size reported by getPayload2, removing the inbound payload
cap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fetching a command from the EA costs two blocking packaged_task
round-trips to the io thread today (GetCommandType + GetCommandPayload).
GetCommandInfo pops the next task and returns its type and payload in
one posted task, halving the cross-thread latency per command.

Queue empty yields command type 0 (MT_COMMAND_TYPE_EMPTY) and an empty
payload. Existing methods are unchanged; everything touching MtExpert
state still runs on the single io_context thread.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New export combining getCommandType and getPayload2 into one call:
pops the next command, writes its type into the by-ref parameter and
copies at most capacity-1 payload characters (always null-terminated).
Returns the full payload length in wchar_t (excluding the terminator)
or -1 on error. Oversized payloads can be re-fetched via the existing
getPayload2, which reads the already popped current task without
popping the queue again.

getCommandType/getPayload/getPayload2 stay exported and functional, so
older experts keep working with the new DLLs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
executeCommand now uses getCommandInfo to receive the command type and
payload together, storing the payload in a global that GetJsonPayload
parses instead of calling back into the DLL. Payloads larger than the
default 5000-char buffer are re-fetched via getPayload2 after
re-allocating. Handlers are untouched; empty payloads still make
GetJsonPayload return NULL with the same errors as before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same change as the MQL5 expert: ExecuteCommand uses getCommandInfo and
GetJsonPayload parses the globally stored payload instead of calling
the DLL again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both compiled with 0 errors, 0 warnings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getCommandInfo pops the command before converting the payload; a
conversion failure (e.g. invalid UTF-8 / non-BMP characters) previously
returned without dispatching, silently dropping the popped command and
leaving the client waiting for the full CommandTimeout. Now the EA
clears the payload and dispatches anyway, so the handler sends the same
immediate "Failed to get payload" error response the old two-call path
produced. Review finding.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant