You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(go): add context variants of blocking calls - #410
The Go binding gets context variants for service calls, service waits, goal sends and subscribers. It also fixes three Go documentation examples that name functions that do not exist. No error code value changes.
Important
This PR needs the fix that allocates pointer message types in the typed subscribers to merge first. It uses the newMessage helper that fix adds. This branch does not build without it.
Key Changes
CallTypedContext makes a service call that ends when the context ends. A context deadline sets the call timeout, up to the 10 second query timeout of the raw service client. A later deadline returns ErrTimeout after about 10 seconds.
ServiceClient.WaitForServiceContext waits for a server until the context ends.
ActionClient.SendGoalContext and GetTypedResultContext add context to goal send and result wait.
BuildWithTypedCallbackContext and SubscriberWithChannelContext bind a subscriber to a context. The second also closes its channel.
Each context call registers with the client before it reads the C handle. Close refuses new calls first and then waits for the registered ones.
The same gate covers GetResultWithContext and its wrappers GetResult, GetTypedResult and GetTypedResultContext. SendGoal and Cancel do not use it.
Close waits for the C call, not for the context. The C call of a result wait times out after 30 seconds. The C call of a goal send times out after 10 seconds. SendGoalContext also stays registered while it runs the goal's SerializeCDR. A slow serializer therefore holds Close for its own time as well. A serializer that calls Close deadlocks. A GetResult call without a deadline holds Close for at most 30 seconds.
Known limit, not new in this PR: GoalHandle.Close is not gated. Do not close a goal handle while a result wait on it is running.
SendGoalContext releases its registration when SerializeCDR panics, and sends nothing when the context ends while it serialises. A panic no longer leaves Close waiting.
Close ends a running WaitForServiceContext within one 250 ms slice.
SendGoalContext and CallTypedContext serialise the message before they start the worker and copy the bytes. The worker owns the copy. A caller can reuse or change the message after a cancelled call, even when its SerializeCDR returns storage that the message owns. The Message interface does not forbid that. The generated SerializeCDR returns a fresh buffer. SendGoal and the plain service calls are unchanged, because they send the bytes before they return.
WaitForServiceContext checks the context and Close again after each C wait. A server that appears while the context ends, or while Close starts, no longer gives success.
A C timeout that returns after the context deadline has passed gives context.DeadlineExceeded, not ErrTimeout.
When the context ends while a result is ready, the call returns ctx.Err(). select picks at random when both channels are ready, so SendGoalContext, GetResultWithContext and CallTypedContext check the context again in the result branch. SendGoalContext closes a handle that arrives after the context ended. CallTypedContext checks the context before it runs SerializeCDR.
SubscriberWithChannelContext waits for running callbacks before it closes the channel.
SubscriberWithChannelContext allocates the message with newMessage[T](). A pointer type such as *std_msgs.String has a nil zero value, and DeserializeCDR on it panics, so the subscriber dropped every message. BuildWithTypedCallbackContext gets the same fix through BuildWithTypedCallback.
docs/bindings/go.md drops client.Call, client.CallWithTimeout, BuildTypedActionClient and SendTypedGoal. None of them exists in the package.
docs/bindings/go.md gains a "Context variants" section.
Before and after
before
after
service call with a context
none
CallTypedContext
service wait with a context
none
WaitForServiceContext
goal send with a context
none
SendGoalContext
subscriber bound to a context
none
two variants
context cancel on a service call
not possible
context.Canceled, not ErrTimeout
goal changed after a cancelled SendGoalContext
the send could read the changed goal
the send uses the goal as it was at the call
goal whose SerializeCDR returns its own storage, changed in place after a cancelled SendGoalContext
the send could read the changed bytes
the send reads its own copy
request whose SerializeCDR returns its own storage, changed in place after a cancelled CallTypedContext
the call could read the changed bytes
the call reads its own copy
WaitForServiceContext when the context ends during a C wait that finds the server
returns success
returns context.Canceled
WaitForServiceContext when Close starts during a C wait that finds the server
returns success
returns the closed-client error
context deadline on a service call
not possible
context.DeadlineExceeded, not ErrTimeout, for a deadline up to 10 seconds
context deadline later than 10 seconds
not possible
ErrTimeout after about 10 seconds, as the docs state
SerializeCDR panics in SendGoalContext
not possible
the registration is released and Close does not wait
server does not answer in time
ErrTimeout
ErrTimeout, unchanged
existing error code values
-1 to -17, -100
unchanged
SubscriberWithChannelContext[*T] delivery
no message arrives, because the receiver is nil
the message arrives
GetResultWithContext exists in 0.2.1. Its registration with the client now happens before the handle read. A call that starts during Close returns an error and does not reach the C layer.
What fails without this
The tests ran with Go 1.26.4 against a debug build of the C library. The new tests use stubs of the C calls and order the goroutines with channels, not sleeps. Three were measured at 167b2677e, the commit that adds them, one at 63d0c88b4 and one at af6493983. Both ran on a scratch merge with the nil-receiver fix.
TestFinishSendGoalEndedContextClosesHandle fails when finishSendGoal does not close the late handle. The other TestFinish* tests have no failing baseline. They call the helpers that hold the result-branch check, and select order is random, so no test fails reliably without the check. TestCallTypedContextCancelledSkipsSerializer does fail without the check before SerializeCDR.
The last row has no failing baseline. The gate tests do not compile against the old code. The old code lets a call register after Close has started waiting. That is a race between two goroutines, and the tests order it with channels, not with sleeps.
The delivery tests for pointer types are in subscriber_context_delivery_test.go. They use a hand-written message type that decodes into its receiver, with a raw-callback control. They were run on the new commit merged with the nil-receiver fix, because this branch alone does not build.
test
new test commit merged with the fix, without the channel fix
passes: the nil-receiver fix already covers BuildWithTypedCallback
passes
The third row has no failing baseline. It pins the delivery path of a function that this branch did not need to change.
Evidence
This branch alone does not compile without the newMessage helper from the nil-receiver fix. The rows below ran on a scratch merge of this branch with that fix. The merge has one textual conflict in the import block of subscriber_typed.go, which takes both import lists. The merge commit is not published.
All rows were measured with this branch at 9cc789a51 as the merge parent, and with the nil-receiver fix as the other merge parent.
what
command
result
go vet with the fix
go vet ./hiroz/
clean
gofmt with the fix
gofmt -l ./hiroz
no file listed
go test -race with the fix
go test -race -v -count=1 ./hiroz/
123 passed, 0 failed, no data race
the context tests, 20 repeats, with the fix
go test -race -count=20 -run 'TestFinish|CancelledSkipsSerializer|SendGoalContext|CallTypedContext' ./hiroz/
ok, 0 failed
TestFinishSendGoalEndedContextClosesHandle without the Close call in finishSendGoal
go test -count=1 -run TestFinishSendGoal ./hiroz/
fails: the handle was not closed
TestCallTypedContextCancelledSkipsSerializer without the check before SerializeCDR
go test -count=1 -run CancelledSkipsSerializer ./hiroz/
fails: the serializer ran for a cancelled call
Each row is one run on Go 1.26.4 with a debug build of the C library. The red result of TestSendGoalContextPanicInSerializeReleasesClient was measured at cd18bbded, the commit that adds it: that test failed. TestSendGoalContextEndedDuringSerializeSendsNothing passed before the fix and only pins the behaviour. The red result of TestSendGoalContextOwnsSerialisedBytes was measured at 63d0c88b4: that test failed and the other tests in the package passed. The red result of TestCallTypedContextOwnsSerialisedBytes was measured at af6493983: that test failed and the other tests in the package passed. The red results of the three earlier new tests were measured at 167b2677e. The other red results were measured at earlier commits of this branch. None of the red results is re-run at the final head. go test ./... also lists the example packages as failed to set up. Their generated message packages are absent in that tree. GOARCH=386 CGO_ENABLED=0 go vet does not apply, because the package needs cgo. Neither run is a result for this change, and neither was re-run at the final head. The documentation sentences about Close and its time bounds were checked by reading the code. No test checks the goal handle cleanup after Close.
Breaking Changes
None. The change adds functions and unexported fields. Existing signatures, error codes and error messages stay as they were. A call on a closed client keeps its error text.
A context call read the C handle before it registered with Close, so Close could free the handle first. Registration now happens first, under the same lock that Close uses to refuse new calls. Close also ends a running wait for a service, a past deadline no longer becomes a huge C timeout, and the channel variant waits for running callbacks before it closes the channel.
GetResult, GetTypedResult and GetTypedResultContext register with the gate through GetResultWithContext. Only SendGoal and Cancel do not. Document the goal handle cleanup after Close as a known limit.
This PR adds context-aware operations to the Go bindings, allowing cancellation of service calls, action waits, and subscriber lifetimes while coordinating client shutdown.
Changes:
Adds context variants for services, actions, and typed subscribers.
Introduces an in-flight call gate and lifecycle regression tests.
Corrects documentation examples and explains cancellation and shutdown limits.
File
Description
docs/bindings/go.md
Corrects examples and documents context variants.
crates/hiroz-go/hiroz/subscriber.go
Adds a replaceable subscriber destruction hook for tests.
crates/hiroz-go/hiroz/subscriber_typed.go
Adds context-bound callback and channel subscribers.
Propagate context timeout to the underlying Zenoh query
crates/hiroz-go/hiroz/service.go:217
A 30-second context deadline cannot receive a response arriving after 15 seconds: the underlying querier has a fixed 10-second timeout (crates/hiroz/src/node.rs:827), so the call returns ErrTimeout early. RawServiceClient.call_raw applies the supplied timeout only to rx.recv_timeout, not to the query. Propagate the selected timeout to the Zenoh query as well so longer context deadlines work as documented.
Honor cancellation before deserializing successful responses
crates/hiroz-go/hiroz/service.go:243
If cancellation occurs before the worker publishes a successful response, both channels can be ready and select can choose the response. Because this context check only runs on errors, CallTypedContext can deserialize the response and return success instead of the documented context error. Check ctx.Err() regardless of r.err, and add coverage for cancellation followed by a successful C response.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Cancellation checks are incomplete, allowing serialization after cancellation and races that return successful responses or goal handles instead of context errors while skipping required handle cleanup.
select picks at random when ctx.Done and the result channel are both ready, so a call could return a result or a goal handle after its context ended. The result branch now checks ctx again. CallTypedContext checks ctx before it runs the serializer.
The handle in the ended-context test has no C handle, so a second Close returns nil whether or not the first ran. Read the close-once guard instead: it is spent only when finishSendGoal called Close. The live-context test checks the guard is unspent.
SendGoalContext stays registered while it runs the goal's SerializeCDR, so a slow serializer holds ActionClient.Close longer than the 10 second C timeout, and a serializer that calls Close deadlocks.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because an unpublished critical callback–teardown race can let consumers release resources before deserialization finishes, violating the promised callback drain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Go binding gets context variants for service calls, service waits, goal sends and subscribers. It also fixes three Go documentation examples that name functions that do not exist. No error code value changes.
Important
This PR needs the fix that allocates pointer message types in the typed subscribers to merge first. It uses the
newMessagehelper that fix adds. This branch does not build without it.Key Changes
CallTypedContextmakes a service call that ends when the context ends. A context deadline sets the call timeout, up to the 10 second query timeout of the raw service client. A later deadline returnsErrTimeoutafter about 10 seconds.ServiceClient.WaitForServiceContextwaits for a server until the context ends.ActionClient.SendGoalContextandGetTypedResultContextadd context to goal send and result wait.BuildWithTypedCallbackContextandSubscriberWithChannelContextbind a subscriber to a context. The second also closes its channel.Closerefuses new calls first and then waits for the registered ones.GetResultWithContextand its wrappersGetResult,GetTypedResultandGetTypedResultContext.SendGoalandCanceldo not use it.Closewaits for the C call, not for the context. The C call of a result wait times out after 30 seconds. The C call of a goal send times out after 10 seconds.SendGoalContextalso stays registered while it runs the goal'sSerializeCDR. A slow serializer therefore holdsClosefor its own time as well. A serializer that callsClosedeadlocks. AGetResultcall without a deadline holdsClosefor at most 30 seconds.GoalHandle.Closeis not gated. Do not close a goal handle while a result wait on it is running.SendGoalContextreleases its registration whenSerializeCDRpanics, and sends nothing when the context ends while it serialises. A panic no longer leavesClosewaiting.Closeends a runningWaitForServiceContextwithin one 250 ms slice.SendGoalContextandCallTypedContextserialise the message before they start the worker and copy the bytes. The worker owns the copy. A caller can reuse or change the message after a cancelled call, even when itsSerializeCDRreturns storage that the message owns. TheMessageinterface does not forbid that. The generatedSerializeCDRreturns a fresh buffer.SendGoaland the plain service calls are unchanged, because they send the bytes before they return.WaitForServiceContextchecks the context andCloseagain after each C wait. A server that appears while the context ends, or whileClosestarts, no longer gives success.context.DeadlineExceeded, notErrTimeout.ctx.Err().selectpicks at random when both channels are ready, soSendGoalContext,GetResultWithContextandCallTypedContextcheck the context again in the result branch.SendGoalContextcloses a handle that arrives after the context ended.CallTypedContextchecks the context before it runsSerializeCDR.SubscriberWithChannelContextwaits for running callbacks before it closes the channel.SubscriberWithChannelContextallocates the message withnewMessage[T](). A pointer type such as*std_msgs.Stringhas a nil zero value, andDeserializeCDRon it panics, so the subscriber dropped every message.BuildWithTypedCallbackContextgets the same fix throughBuildWithTypedCallback.docs/bindings/go.mddropsclient.Call,client.CallWithTimeout,BuildTypedActionClientandSendTypedGoal. None of them exists in the package.docs/bindings/go.mdgains a "Context variants" section.Before and after
CallTypedContextWaitForServiceContextSendGoalContextcontext.Canceled, notErrTimeoutSendGoalContextSerializeCDRreturns its own storage, changed in place after a cancelledSendGoalContextSerializeCDRreturns its own storage, changed in place after a cancelledCallTypedContextWaitForServiceContextwhen the context ends during a C wait that finds the servercontext.CanceledWaitForServiceContextwhenClosestarts during a C wait that finds the servercontext.DeadlineExceeded, notErrTimeout, for a deadline up to 10 secondsErrTimeoutafter about 10 seconds, as the docs stateSerializeCDRpanics inSendGoalContextClosedoes not waitErrTimeoutErrTimeout, unchangedSubscriberWithChannelContext[*T]deliveryGetResultWithContextexists in 0.2.1. Its registration with the client now happens before the handle read. A call that starts duringClosereturns an error and does not reach the C layer.What fails without this
The tests ran with Go 1.26.4 against a debug build of the C library. The new tests use stubs of the C calls and order the goroutines with channels, not sleeps. Three were measured at
167b2677e, the commit that adds them, one at63d0c88b4and one ataf6493983. Both ran on a scratch merge with the nil-receiver fix.TestCallTypedContextTimeoutAfterDeadlineIsDeadlineExceededErrTimeoutTestSubscriberWithChannelContextLateCallbackIsSafeTestSendGoalContextSerialisesBeforeReturnTestSendGoalContextOwnsSerialisedBytes[9 2]where the goal was[1 2]at the callTestCallTypedContextOwnsSerialisedBytes[9 2]where the request was[1 2]at the callTestWaitForServiceContextCancelDuringWaitWinsOverReadyTestWaitForServiceContextCloseDuringWaitWinsOverReadySendGoalContextlifetime testsTestCallTypedContextRefusedOnceCloseStarted,TestSendGoalContextRefusedOnceCloseStarted,TestCloseEndsWaitForServiceContext,TestWaitSliceMsTestFinishSendGoalEndedContextClosesHandlefails whenfinishSendGoaldoes not close the late handle. The otherTestFinish*tests have no failing baseline. They call the helpers that hold the result-branch check, andselectorder is random, so no test fails reliably without the check.TestCallTypedContextCancelledSkipsSerializerdoes fail without the check beforeSerializeCDR.The last row has no failing baseline. The gate tests do not compile against the old code. The old code lets a call register after
Closehas started waiting. That is a race between two goroutines, and the tests order it with channels, not with sleeps.The delivery tests for pointer types are in
subscriber_context_delivery_test.go. They use a hand-written message type that decodes into its receiver, with a raw-callback control. They were run on the new commit merged with the nil-receiver fix, because this branch alone does not build.TestCtxDeliveryRawCallbackControlTestSubscriberWithChannelContextDeliversForPointerTypeTestBuildWithTypedCallbackContextDeliversForPointerTypeBuildWithTypedCallbackThe third row has no failing baseline. It pins the delivery path of a function that this branch did not need to change.
Evidence
This branch alone does not compile without the
newMessagehelper from the nil-receiver fix. The rows below ran on a scratch merge of this branch with that fix. The merge has one textual conflict in the import block ofsubscriber_typed.go, which takes both import lists. The merge commit is not published.All rows were measured with this branch at
9cc789a51as the merge parent, and with the nil-receiver fix as the other merge parent.go vetwith the fixgo vet ./hiroz/gofmtwith the fixgofmt -l ./hirozgo test -racewith the fixgo test -race -v -count=1 ./hiroz/go test -race -count=20 -run 'TestFinish|CancelledSkipsSerializer|SendGoalContext|CallTypedContext' ./hiroz/TestFinishSendGoalEndedContextClosesHandlewithout theClosecall infinishSendGoalgo test -count=1 -run TestFinishSendGoal ./hiroz/TestCallTypedContextCancelledSkipsSerializerwithout the check beforeSerializeCDRgo test -count=1 -run CancelledSkipsSerializer ./hiroz/Each row is one run on Go 1.26.4 with a debug build of the C library. The red result of
TestSendGoalContextPanicInSerializeReleasesClientwas measured atcd18bbded, the commit that adds it: that test failed.TestSendGoalContextEndedDuringSerializeSendsNothingpassed before the fix and only pins the behaviour. The red result ofTestSendGoalContextOwnsSerialisedByteswas measured at63d0c88b4: that test failed and the other tests in the package passed. The red result ofTestCallTypedContextOwnsSerialisedByteswas measured ataf6493983: that test failed and the other tests in the package passed. The red results of the three earlier new tests were measured at167b2677e. The other red results were measured at earlier commits of this branch. None of the red results is re-run at the final head.go test ./...also lists the example packages as failed to set up. Their generated message packages are absent in that tree.GOARCH=386 CGO_ENABLED=0 go vetdoes not apply, because the package needs cgo. Neither run is a result for this change, and neither was re-run at the final head. The documentation sentences aboutCloseand its time bounds were checked by reading the code. No test checks the goal handle cleanup afterClose.Breaking Changes
None. The change adds functions and unexported fields. Existing signatures, error codes and error messages stay as they were. A call on a closed client keeps its error text.