Skip to content

Commit e075d38

Browse files
s-cellesclaude
andcommitted
Fix streaming hangs and enable streaming on Julia 1.10 (#68)
`src/Streaming.jl` was compiled out below Julia 1.12 because streaming calls hung there. The hang reproduces on a bidirectional stream that moves ~1000 messages with `-t 4`, roughly one run in three on Julia 1.10, and it is not a Julia version problem: the same wedge, plus a second one at end of stream, also reproduces on 1.12 given enough runs. Both come from the same place. read_callback pauses libcurl's send direction whenever the request buffer runs dry, and each `curl_easy_pause(CURLPAUSE_CONT)` the request pump issues to resume it is one-shot. libcurl can drop a transfer on the floor around one of those un-pauses: it stops servicing the transfer's socket entirely, so the peer's response bytes pile up unread in the receive queue - 150KB of them in one captured hang, exactly the responses the caller was still waiting for - while `curl_multi_socket_action` keeps reporting CURLM_OK without consuming any. Nothing but another un-pause recovers it; driving the multi handle, by socket event or by timeout, does not. So a caller that waits on responses before sending more deadlocks until its deadline. An in-flight request-streaming call now re-issues the un-pause every 50ms while libcurl holds none of its request data, instead of relying on a single one landing. `curl_easy_pause` returns immediately when the transfer is not paused, and an un-pause with an empty request buffer just draws one read_callback that pauses straight back, so a healthy call pays only for the call itself. The end-of-stream path had the mirror-image bug, and it was reachable on every version: it un-paused and only then closed `req.request_c` in its `finally`, so a read_callback landing in between still saw an open stream, paused again, and left the transfer paused with nobody to un-pause it - all data exchanged, both peers done, call hung until the deadline. It now closes the stream first, so the callback the un-pause draws can only take its end-of-stream branch. Streaming is enabled on all supported versions, the version guard around the streaming testsets is gone, and the docs no longer describe streaming as 1.12 only. "Don't Stick User Tasks" is marked broken below 1.12: it is a unary test that only sat in the guarded block, and the stickiness comes from arming the deadline watchdog with `Timer(cb, delay)`, whose `@async` marks the scheduling task sticky on those versions (JuliaLang/julia#41324). Verified against the Go test server on 1.10.11 (libcurl 8.4.0) and 1.12.6 (libcurl 8.15.0): 1000 bidirectional stress runs across both versions and both close orderings with no hang, the full suite 12 times per version plus `-t 1`, `-t 2` and `-t 8` on 1.10, and Runic clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 440c5fc commit e075d38

8 files changed

Lines changed: 420 additions & 362 deletions

File tree

CONTRIBUTING.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ JULIA_GRPCCLIENT_TEST_START_SERVER=go julia --project test/runtests.jl
6060
If you already have a server listening on `localhost:8001`, plain `julia --project test/runtests.jl`
6161
is enough. `GRPC_TEST_SERVER_HOST` and `GRPC_TEST_SERVER_PORT` point the suite elsewhere.
6262

63-
CI covers Julia 1.10, 1.12, and nightly across Linux, Windows, and macOS. Note that
64-
`src/Streaming.jl` is compiled only on Julia 1.12 and newer, so streaming code and its tests need
65-
matching version guards.
63+
CI covers Julia 1.10, 1.12, and nightly across Linux, Windows, and macOS.
6664

6765
## Pull requests
6866

skills/grpcclient-jl-dev/SKILL.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,14 @@ description: Work on the gRPCClient.jl package itself. Covers running the Go tes
1313
| `src/Curl.jl` | The libcurl transport: `gRPCCURL`, `gRPCRequest`, callbacks, socket watchers, concurrency semaphore, deadline watchdog, `grpc_cancel` |
1414
| `src/gRPC.jl` | Public handle lifecycle, `gRPCServiceClient`, request framing, the generic `grpc_async_await` |
1515
| `src/Unary.jl` | Unary methods and `gRPCAsyncChannelResponse` |
16-
| `src/Streaming.jl` | Streaming methods and the request and response pump tasks; included only on Julia 1.12 and newer |
16+
| `src/Streaming.jl` | Streaming methods and the request and response pump tasks |
1717
| `src/ProtoBuf.jl` | The ProtoBuf.jl codegen hook that emits `*_Client` constructors |
1818
| `test/proto/test.proto` | One service with all four RPC variants |
1919
| `test/gen/` | Checked-in generated stubs used by the suite |
2020
| `test/go/` | The Go reference server every test and benchmark runs against |
2121
| `test/python/` | Generated Python stubs, regenerated alongside the Julia ones |
2222
| `utils/gRPCClientUtils.jl` | Separate package for benchmarks, stress workloads, and memory profiling |
2323

24-
Streaming is conditionally compiled: `src/Streaming.jl` is included only under `@static if VERSION >= v"1.12"`, so anything added there needs a matching version guard in the tests.
25-
2624
## Formatting
2725

2826
The repository is formatted with [Runic.jl](https://github.com/fredrikekre/Runic.jl), and contributed code is expected to be run through it before a pull request goes up:

skills/grpcclient-jl/SKILL.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ description: Call gRPC services from Julia with gRPCClient.jl. Covers generating
55

66
# gRPCClient.jl
77

8-
A Julia gRPC client built on libcurl for HTTP/2 transport, with client stubs generated by ProtoBuf.jl. Version 1.0.4. Requires Julia 1.10; streaming RPC requires Julia 1.12 or newer and is compiled out below that.
8+
A Julia gRPC client built on libcurl for HTTP/2 transport, with client stubs generated by ProtoBuf.jl. Version 1.0.4. Requires Julia 1.10.
99

1010
Exported names: `grpc_init`, `grpc_shutdown`, `grpc_global_handle`, `grpc_register_service_codegen`, `grpc_async_request`, `grpc_async_await`, `grpc_sync_request`, `grpc_cancel`, `gRPCCURL`, `gRPCRequest`, `gRPCServiceClient`, `gRPCAsyncChannelResponse`, `gRPCException`, `gRPCServiceCallException`.
1111

@@ -89,7 +89,7 @@ The channel form never throws at submission time; a failed request arrives as a
8989

9090
## Streaming RPC
9191

92-
Requires Julia 1.12 or newer. Requests and responses move through `Channel`s, and the streaming side of the call is chosen by the RPC definition, not by the call site:
92+
Requests and responses move through `Channel`s, and the streaming side of the call is chosen by the RPC definition, not by the call site:
9393

9494
| RPC variant | Call shape | `grpc_async_await` |
9595
|---|---|---|
@@ -131,7 +131,6 @@ Which call raises matters when writing error handling. `grpc_async_request` thro
131131
| `MethodError` on a keyword named `max_receive_message_length` | The keyword is `max_recieve_message_length` |
132132
| `ArgumentError: Invalid field of gRPCConnectionOptions: x` | Unknown per-call option keyword, most often a misspelling |
133133
| `DEADLINE_EXCEEDED` under load with a healthy server | The 10 second default deadline includes time queued for one of the handle's 16 concurrency slots; see `references/concurrency.md` |
134-
| Streaming functions undefined | Julia is older than 1.12; a warning was emitted at load |
135134
| A client or bidi stream never finishes | The request channel was never closed, so the server is still waiting for end of stream |
136135
| `INVALID_ARGUMENT` mentioning the authorization header | `token` and an `authorization` metadata entry were both set |
137136
| Call hangs forever | `deadline = Inf` with no `grpc_cancel`, which is the documented behavior of that setting |

skills/grpcclient-jl/references/streaming.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
# Streaming RPC
22

3-
Requires Julia 1.12 or newer. On older versions `src/Streaming.jl` is not included at all, a warning is emitted at load time, and the streaming `grpc_async_request` methods simply do not exist, so the failure looks like a `MethodError` rather than a version check.
4-
53
Every streaming variant follows the same rhythm: create the channels, start the request, move messages, then await for errors. `grpc_async_request` returns a `gRPCRequest` immediately in all three cases.
64

75
## Client streaming, many requests to one response

src/Curl.jl

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,11 @@ end
343343
# watchdog only wins when libcurl has wedged (see the watchdog comment in gRPCRequest).
344344
const GRPC_DEADLINE_GRACE = 0.25
345345

346+
# How often (seconds) an in-flight request-streaming call re-issues
347+
# `curl_easy_pause(CURLPAUSE_CONT)` while libcurl has our request buffer drained. See
348+
# the `unpause` field in gRPCRequest for why this is needed.
349+
const GRPC_UNPAUSE_INTERVAL = 0.05
350+
346351
mutable struct gRPCRequest
347352
# CURL multi lock for exclusive access to the easy handle after its added to the multi
348353
lock::ReentrantLock
@@ -407,6 +412,30 @@ mutable struct gRPCRequest
407412
# Client-side deadline watchdog, see the comment in the constructor
408413
timer::Union{Nothing, Timer}
409414

415+
# Repeating un-pause for a request-streaming call, `nothing` for every other kind.
416+
#
417+
# read_callback pauses libcurl's send direction whenever the request buffer runs
418+
# dry, which for a streaming request is every time the caller has nothing queued,
419+
# and `curl_easy_pause(CURLPAUSE_CONT)` from the request pump is what resumes it.
420+
# Every un-pause the pump issues is one-shot, and libcurl can drop the transfer on
421+
# the floor around one: it stops servicing the transfer's socket entirely, so the
422+
# peer's response bytes pile up unread in the receive queue while
423+
# curl_multi_socket_action keeps reporting CURLM_OK without consuming any of them.
424+
# Nothing but another un-pause shakes it loose - driving the multi handle, by socket
425+
# event or by timeout, does not - so a bidirectional stream whose caller is waiting
426+
# on responses before it sends anything more deadlocks until its deadline expires.
427+
# This is what made streaming unusable on Julia 1.10, whose LibCURL_jll is pinned to
428+
# the 8.4 series: https://github.com/JuliaIO/gRPCClient.jl/issues/68
429+
#
430+
# So rather than rely on a single un-pause landing, keep re-issuing it for as long as
431+
# such a call is in flight and libcurl holds none of our request data. That covers
432+
# the end of the stream too, where the un-pause is what draws the read_callback that
433+
# returns 0 and closes the request. curl_easy_pause returns immediately without
434+
# doing anything when the transfer is not paused, and an un-pause with an empty
435+
# request buffer just draws one read_callback that pauses straight back, so a healthy
436+
# call pays only for the call itself while a stalled one recovers within an interval.
437+
unpause::Union{Nothing, Timer}
438+
410439
# Absolute `time()` at which this request's deadline expires, Inf when there is no
411440
# deadline. grpc_async_await uses it to attribute a transport error that landed after
412441
# the deadline to DEADLINE_EXCEEDED rather than INTERNAL: when libcurl tears a
@@ -637,10 +666,37 @@ mutable struct gRPCRequest
637666
grpc,
638667
false,
639668
watchdog,
669+
nothing,
640670
expiry,
641671
)
642672
preserve_handle(req)
643673

674+
# Arm the repeating un-pause once `req` exists, see the `unpause` field above.
675+
# Only a request-streaming call ever pauses, so nothing else needs one.
676+
if isstreaming_request(req)
677+
req.unpause = Timer(
678+
GRPC_UNPAUSE_INTERVAL;
679+
interval = GRPC_UNPAUSE_INTERVAL,
680+
) do _
681+
try
682+
lock(grpc.lock) do
683+
# cleanup_request runs under this lock and frees the easy
684+
# handle, so re-check both before touching it
685+
(req.completed || !grpc.running) && return
686+
# Only once libcurl has taken every byte of the request buffer,
687+
# which is exactly when there is nothing in flight for the
688+
# un-pause to disturb. While the pump has a batch staged the
689+
# buffer is non-empty, so this stays out of the handoff the pump
690+
# is in the middle of driving itself.
691+
req.request_ptr == req.request.size &&
692+
curl_easy_pause(req.easy, CURLPAUSE_CONT)
693+
end
694+
catch err
695+
@error("streaming un-pause: unexpected error", err, maxlog = 1_000)
696+
end
697+
end
698+
end
699+
644700
req_p = pointer_from_objref(req)
645701
curl_easy_setopt(easy_handle, CURLOPT_PRIVATE, req_p)
646702

@@ -671,6 +727,7 @@ mutable struct gRPCRequest
671727
# shut-down handle is a submission-time FAILED_PRECONDITION per the
672728
# contract at the top of this constructor.
673729
isnothing(watchdog) || close(watchdog)
730+
isnothing(req.unpause) || close(req.unpause)
674731
curl_easy_cleanup(easy_handle)
675732
curl_slist_free_all(headers)
676733
unpreserve_handle(req)
@@ -747,6 +804,7 @@ mutable struct gRPCRequest
747804
grpc,
748805
true,
749806
nothing,
807+
nothing,
750808
expiry,
751809
)
752810

@@ -1307,6 +1365,9 @@ function cleanup_request(grpc::gRPCCURL, req::gRPCRequest)
13071365
req.completed = true
13081366
# Stop the deadline watchdog
13091367
isnothing(req.timer) || close(req.timer)
1368+
# Stop the repeating streaming un-pause. It is a *repeating* Timer, so leaving it
1369+
# open would keep the libuv event loop alive for the rest of the process.
1370+
isnothing(req.unpause) || close(req.unpause)
13101371
# First remove from the multi
13111372
curl_multi_remove_handle(grpc.multi, req.easy)
13121373
# Cleanup the easy handle

src/Streaming.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,15 @@ function grpc_async_stream_request(
6868
# wakes; the re-check under req.lock keeps curl_easy_pause off the freed
6969
# easy handle).
7070
if !req.completed
71+
# Mark the request stream closed before the un-pause below, not just in
72+
# the `finally` further down. read_callback only returns 0 (which is what
73+
# ends the request) once it sees this closed, and the un-pause is what
74+
# draws that callback: with the close left until afterwards, a callback
75+
# landing in between still sees an open stream, pauses again, and the
76+
# transfer sits paused with nobody left to un-pause it, so the call hangs
77+
# until its deadline even though both sides are done.
78+
close(req.request_c)
79+
7180
# Wait for any request data to be flushed by curl
7281
wait(req.curl_done_reading)
7382

src/gRPCClient.jl

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,7 @@ include("Curl.jl")
8585
include("gRPC.jl")
8686
include("Unary.jl")
8787

88-
# Streaming only supported on >= 1.12
89-
@static if VERSION >= v"1.12"
90-
include("Streaming.jl")
91-
else
92-
@warn "Julia $(VERSION) <= 1.12, streaming support is disabled: https://github.com/JuliaIO/gRPCClient.jl/issues/68"
93-
end
94-
88+
include("Streaming.jl")
9589
include("ProtoBuf.jl")
9690

9791
export grpc_init

0 commit comments

Comments
 (0)