JohnSColeman commented on code in PR #10:
URL:
https://github.com/apache/ignite-nodejs-thin-client/pull/10#discussion_r3468479292
##########
src/internal/ClientSocket.ts:
##########
@@ -296,15 +311,40 @@ export default class ClientSocket {
if (this._requests.has(requestId)) {
const request = this._requests.get(requestId);
this._requests.delete(requestId);
+
+ // Carve a fresh, independent MessageBuffer from just this
message's
+ // payload bytes (after length field + request-id). Passing a
copy
+ // rather than the shared socket buffer prevents two cursors
created
+ // from the same TCP segment from aliasing the same position
pointer
+ // and corrupting each other's reads under parallel scan
workloads.
+ // Built only on the matched-request path so unmatched frames
cost no copy.
+ const headerConsumed = isHandshake
+ ? BinaryUtils.getSize(BinaryUtils.TYPE_CODE.INTEGER)
// 4 B: length only
+ : BinaryUtils.getSize(BinaryUtils.TYPE_CODE.INTEGER) +
// 4 B: length
+ BinaryUtils.getSize(BinaryUtils.TYPE_CODE.LONG);
// 8 B: request-id
+ const freshBuffer = MessageBuffer.from(
+ buffer.getSlice(msgStart + headerConsumed, msgEnd),
+ 0
+ );
Review Comment:
`getSlice(start, end)` wraps `Buffer.slice(start, end)`, so the second
argument is an end offset, not a length — `getSlice(msgStart + headerConsumed,
msgEnd)` is correct. The `getSlice(this._offset - length, length)` call
referenced here was itself the bug fixed in this PR (the `_logMessage`
empty-bytes issue, now `getSlice(msgStart, msgEnd)`), so it is not a valid
reference for the intended contract. Applying the suggested change (passing a
computed length as the second arg) would slice too few bytes for any 2nd+ frame
in a TCP segment and actually corrupt parsing. Covered by the passing scan/SQL
cursor suites, which deserialize multi-row pages directly out of `freshBuffer`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
