Azure / Azure/azure-sdk-for-cpp

core-amqp: bound the remaining block_on calls in the Rust wrapper

Open
#7,348 0 comments 0 reactions 0 assignees View on GitHub
Azure.Core bug Client
Dominant language
C++
Stars
205
Forks
172
Avg merge
1d 15m
Merged PRs (30d)
33

Description

## Summary

Nine FFI entry points in the Rust wrapper call `runtime().block_on(...)` directly, so each one ignores the per-call bound that `RustCallContext` carries. Any of them can hold the calling thread forever when the operation never completes.

## Motivation

PR #7346 added the `block_on_with_timeout` helper and the `call_context_set_timeout_ms` FFI setter, then routed the two connection close entry points through the helper. The other entry points still block with no bound. A connection that the service dropped makes each of them wait forever, which is the same defect that #7346 fixed for the close path. A caller that sets a deadline gets no benefit today, because the Rust side never reads the bound on these paths.

## Proposal

Route each call site through `call_context.block_on_with_timeout(...)`, and match `Ok(Ok(_))`, `Ok(Err(err))`, and `Err(elapsed)` the same way `amqpconnection_close` does.

- `amqpconnection_open`, `src/amqp/connection.rs:103`
- `amqpsession_begin`, `src/amqp/session.rs:65`
- `amqpsession_begin_with_options`, `src/amqp/session.rs:79`
- `amqpmanagement_attach`, `src/amqp/management.rs:112`
- `amqpmanagement_detach`, `src/amqp/management.rs:140`
- `amqpmanagement_call`, `src/amqp/management.rs:179`
- `amqpclaimsbasedsecurity_attach`, `src/amqp/cbs.rs:77`
- `amqpclaimsbasedsecurity_detach`, `src/amqp/cbs.rs:107`
- `amqpclaimsbasedsecurity_authorize_path`, `src/amqp/cbs.rs:179`

Make each matching C++ caller set the bound with `CallContext::SetTimeoutMilliseconds` where it does not already.

The paths are relative to `sdk/core/azure-core-amqp/src/impl/rust_amqp/rust_amqp/rust_wrapper`. The block_on call at `src/amqp/message_receiver.rs:148` needs no bound, because it waits only for a task spawn and returns at once. Three further call sites sit inside `mod tests`.

Found while reviewing #7346.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.