Skip to content

Commit 21ea9d1

Browse files
JCMaisJCMais
committed
fix: track CURLPAUSE_SEND state for mime read callback pause
The mime StaticReadCallback was returning CURL_READFUNC_PAUSE without updating pauseState, so isPausedSend/isPausedRecv both stayed false. This caused the test's conditional unpause (which checks pause state before calling curl_easy_pause) to be a silent no-op, leaving the transfer paused forever once libcurl needed a second read callback. The test only passed when the stream's 'end' event fired before libcurl's second read attempt — a race that resolved differently across Node.js versions and platforms. Additionally, the docs and examples incorrectly referenced CurlPause.Recv. The mime read callback supplies upload data, so it pauses SEND (matching Easy::ReadFunction). Updated docs, examples, and tests to use Send. Also reverted setTimeout back to setImmediate now that the underlying race is properly fixed.
1 parent 50cdc77 commit 21ea9d1

4 files changed

Lines changed: 34 additions & 30 deletions

File tree

‎lib/CurlMimePart.ts‎

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export interface MimeDataCallbacks {
2424
*
2525
* @remarks
2626
* When `CurlReadFunc.Pause` is returned, the transfer will be paused until it is
27-
* explicitly resumed by calling `handle.pause(handle.pauseFlags & ~CurlPause.Recv)`.
27+
* explicitly resumed by calling `handle.pause(handle.pauseFlags & ~CurlPause.Send)`.
2828
* When `CurlReadFunc.Abort` is returned, the transfer will be aborted.
2929
*
3030
* @example
@@ -360,8 +360,9 @@ declare class CurlMimePart {
360360
* `CurlReadFunc.Pause`, and the `unpause` callback is invoked when data becomes
361361
* available to resume the transfer.
362362
*
363-
* The `unpause` function should unpause the curl handle's receive operation, typically
364-
* by calling `handle.pause(handle.pauseFlags & ~CurlPause.Recv)`.
363+
* The `unpause` function should unpause the curl handle's send operation (mime upload
364+
* data is sent via the read callback), typically by calling
365+
* `handle.pause(handle.pauseFlags & ~CurlPause.Send)`.
365366
*
366367
* For very large files, consider using {@link setFileData} instead, as it streams
367368
* directly from disk without going through Node.js streams.
@@ -380,7 +381,7 @@ declare class CurlMimePart {
380381
* .addPart()
381382
* .setName('document')
382383
* .setDataStream(stream, () => {
383-
* curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
384+
* curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
384385
* })
385386
* .setType('text/plain')
386387
* ```
@@ -402,7 +403,7 @@ declare class CurlMimePart {
402403
* .setName('document')
403404
* .setDataStream(
404405
* stream,
405-
* () => curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv),
406+
* () => curl.pause(curl.handle.pauseFlags & ~CurlPause.Send),
406407
* size
407408
* )
408409
* ```
@@ -426,22 +427,17 @@ CurlMimePart.prototype.setDataStream = function (
426427
let streamError: Error | null = null
427428
let paused = false
428429

429-
// Defer unpause to a later event loop iteration to avoid calling
430+
// Defer unpause to the next event loop iteration to avoid calling
430431
// curl_easy_pause() while libcurl is still processing the READFUNC_PAUSE
431432
// return value from the read callback. Without this, the synchronous
432433
// unpause can re-enter libcurl and cause a hang (observed on Linux).
433-
//
434-
// We use setTimeout(fn, 0) rather than setImmediate because setTimeout
435-
// fires in the timer phase of the event loop, which is the same phase
436-
// where libcurl's multi handle timeout callback fires. This ensures the
437-
// unpause is processed in a compatible event loop phase across all
438-
// Node.js versions.
434+
// This matches the pattern used by setUploadStream in Curl.ts.
439435
const deferredUnpause = () => {
440436
if (paused) {
441437
paused = false
442-
setTimeout(() => {
438+
setImmediate(() => {
443439
unpause()
444-
}, 0)
440+
})
445441
}
446442
}
447443

‎lib/Easy.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,8 @@ const Easy = bindings.Easy as Easy
745745
* @remarks
746746
* For stream-based parts, you must provide the unpause callback that will be
747747
* called when more data is available. The callback should unpause the transfer
748-
* using `handle.pause(handle.pauseFlags & ~CurlPause.Recv)`.
748+
* using `handle.pause(handle.pauseFlags & ~CurlPause.Send)` (mime upload data
749+
* is sent via the read callback, so it pauses SEND, not RECV).
749750
*
750751
* Available since libcurl 7.56.0.
751752
*
@@ -774,7 +775,7 @@ const Easy = bindings.Easy as Easy
774775
* name: 'logfile',
775776
* stream: createReadStream('/path/to/log.txt'),
776777
* unpause: () => {
777-
* easy.pause(easy.pauseFlags & ~CurlPause.Recv)
778+
* easy.pause(easy.pauseFlags & ~CurlPause.Send)
778779
* },
779780
* size: 12345
780781
* },
@@ -832,7 +833,7 @@ Easy.prototype.setMimePost = function (
832833
part.setDataStream(
833834
partSpec.stream,
834835
() => {
835-
this.pause(this.pauseFlags & ~CurlPause.Recv)
836+
this.pause(this.pauseFlags & ~CurlPause.Send)
836837
},
837838
partSpec.size,
838839
)

‎src/CurlMime.cc‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,14 @@ size_t CurlMimePart::StaticReadCallback(char* buffer, size_t size, size_t nitems
595595
}
596596

597597
if (result.IsNumber()) {
598-
return result.As<Napi::Number>().Int32Value();
598+
int32_t returnValue = result.As<Napi::Number>().Int32Value();
599+
// Track pause state so isPausedSend reflects reality.
600+
// The mime data callback pauses SEND (it supplies upload data),
601+
// matching the behavior of Easy::ReadFunction.
602+
if (returnValue == CURL_READFUNC_PAUSE) {
603+
part->easy->pauseState |= CURLPAUSE_SEND;
604+
}
605+
return static_cast<size_t>(returnValue);
599606
}
600607

601608
// Invalid return type

‎test/curl/CurlMime.spec.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -813,8 +813,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
813813
.addPart()
814814
.setName('stream_field')
815815
.setDataStream(stream, () => {
816-
if (curl.handle.isPausedRecv) {
817-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
816+
if (curl.handle.isPausedSend) {
817+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
818818
}
819819
})
820820

@@ -863,8 +863,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
863863
.setDataStream(
864864
stream,
865865
() => {
866-
if (curl.handle.isPausedRecv) {
867-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
866+
if (curl.handle.isPausedSend) {
867+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
868868
}
869869
},
870870
testData.length,
@@ -913,8 +913,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
913913
.addPart()
914914
.setName('buffer_stream')
915915
.setDataStream(stream, () => {
916-
if (curl.handle.isPausedRecv) {
917-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
916+
if (curl.handle.isPausedSend) {
917+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
918918
}
919919
})
920920

@@ -961,8 +961,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
961961
.addPart()
962962
.setName('chained_stream')
963963
.setDataStream(stream, () => {
964-
if (curl.handle.isPausedRecv) {
965-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
964+
if (curl.handle.isPausedSend) {
965+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
966966
}
967967
})
968968
.setType('text/plain')
@@ -1012,8 +1012,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
10121012
.addPart()
10131013
.setName('chunked_stream')
10141014
.setDataStream(stream, () => {
1015-
if (curl.handle.isPausedRecv) {
1016-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
1015+
if (curl.handle.isPausedSend) {
1016+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
10171017
}
10181018
})
10191019

@@ -1058,8 +1058,8 @@ describe.runIf(Curl.isVersionGreaterOrEqualThan(7, 56, 0))('CurlMime', () => {
10581058
.addPart()
10591059
.setName('empty_stream')
10601060
.setDataStream(stream, () => {
1061-
if (curl.handle.isPausedRecv) {
1062-
curl.pause(curl.handle.pauseFlags & ~CurlPause.Recv)
1061+
if (curl.handle.isPausedSend) {
1062+
curl.pause(curl.handle.pauseFlags & ~CurlPause.Send)
10631063
}
10641064
})
10651065

0 commit comments

Comments
 (0)