[birschick-bq]

THRIFT-5830: Using THttpTransport using interleaved asynchronous calls may throw exception
Client: netstd

feat: Implement per-call transport support in THttpTransport

• Added ITPerCallTransportProvider interface to define per-call transport creation.
• Enhanced TBaseClient to support per-call transport with separate protocols.
• Introduced THttpPerCallTransport for handling HTTP requests on a per-call basis.
• Updated THttpTransport to manage lifecycle and creation of per-call transports.
• Improved error handling and resource management in transport classes.
• Updated tutorial with per-call semantics. Note: requires new version of Thrift.exe

Co-authored-by: Claude Sonnet 5
Co-authored-by: GPT 5.6 Luna
Co-authored-by: GitHub Copilot

• Did you create an Apache Jira ticket? (Request account here, not required for trivial changes)
• If a ticket exists: Does your pull request title follow the pattern "THRIFT-NNNN: describe my issue"?
• Did you squash your changes to a single commit? (not required, but preferred)
• Did you do your best to avoid breaking changes? If one was needed, did you label the Jira ticket with "Breaking-Change"?
• If your change does not involve any code, include [skip ci] anywhere in the commit message to free up build resources.

[Copilot]

Copilot review overview

🟡 Changes recommended

Legacy .NET response reads can still block beyond configured deadlines, and the shared path may use a stale timeout.

Review effort: Balanced
Findings: 2 · 1

Open (3)

• Per-call legacy body reads can block despite response deadline · New
• Legacy response reads ignore cancellation and can stall past timeout · New
• Shared reads use stale timeout instead of HttpClient.Timeout · New

What changed in this PR

Adds isolated per-call HTTP transports so generated .NET clients can safely perform interleaved asynchronous RPCs.

Changes:

• Adds per-call transport creation and lifecycle management.
• Updates client generation and protocol scoping for concurrent calls.
• Adds concurrency, timeout, cancellation, and disposal tests.

File	Description
lib/​netstd/​Thrift/​Transport/​ITPerCallTransportProvider.cs	Defines per-call transport creation.
lib/​netstd/​Thrift/​Transport/​Client/​THttpTransport.cs	Manages shared HTTP resources and per-call transports.
lib/​netstd/​Thrift/​Transport/​Client/​THttpPerCallTransport.cs	Implements isolated request/response state.
lib/​netstd/​Thrift/​TBaseClient.cs	Scopes protocols to each asynchronous call.
lib/​netstd/​Tests/​Thrift.Tests/​Transports/​THttpTransportTests.cs	Tests concurrency and lifecycle behavior.
compiler/​cpp/​src/​thrift/​generate/​t_netstd_generator.cc	Generates constructors and per-call execution wrappers.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Response-body timeouts remain ineffective on .NET Framework for both HTTP read paths.

Review effort: Balanced
Findings: 2

Open (2)

• Legacy response reads ignore cancellation and can stall past timeout
• Per-call legacy body reads can block despite response deadline

Resolved since last review (1)

• Shared reads use stale timeout instead of HttpClient.Timeout

[Copilot]

Copilot review overview

🟡 Changes recommended

The commit message must use the repository-required THRIFT-5830: subject and Client: netstd line.

Review effort: Balanced
Findings: 1 · 1

Open (2)

• Per-call legacy body reads can block despite response deadline
• Fix commit message format and add Client: netstd · New

Resolved since last review (1)

• Legacy response reads ignore cancellation and can stall past timeout

[Copilot]

Copilot review overview

🟡 Changes recommended

The new legacy .NET cancellation branch lacks runtime test coverage.

Review effort: Balanced
Findings: 2

Open (2)

• Add runtime test for legacy cancellation stream disposal · New
• Fix commit message format and add Client: netstd

Resolved since last review (1)

• Per-call legacy body reads can block despite response deadline

[Copilot]

Copilot review overview

🟡 Changes recommended

The public provider path does not validate null transports and can mask failures with a secondary exception.

Review effort: Balanced
Findings: 1

Open (1)

• Validate null transport provider results before disposal · New

Resolved since last review (2)

• Add runtime test for legacy cancellation stream disposal
• Fix commit message format and add Client: netstd

[Copilot]

Copilot review overview

🟡 Changes recommended

The close/write race and commit-format issue should be resolved before approval.

Review effort: Balanced
Findings: 1

Open (1)

• Squash commits and restore required issue, client, and attribution format · New

Resolved since last review (1)

• Validate null transport provider results before disposal

Previously missed (1)

In code that hasn't changed since last review

Normalize stream-close race to the transport NotOpen error

lib/​netstd/​Thrift/​Transport/​Client/​THttpTransport.cs:293

Close() can dispose this captured stream after the lifecycle lock is released but before WriteAsync starts, causing a raw ObjectDisposedException instead of the transport's NotOpen error. Normalize that close race as the other I/O paths do.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Cross-target cancellation and concurrent resource-lifecycle behavior warrant final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Squash commits and restore required issue, client, and attribution format

[birschick-bq]

@Jens-G
I've addressed the comments in the comment on the abandoned PR
#3930 (comment)

[birschick-bq]

abandoning this change.

[Jens-G]

I didnt have the time to look into it yet. You're giving up on this or you simply don't want to wait? @birschick-bq

[birschick-bq]

I didnt have the time to look into it yet. You're giving up on this or you simply don't want to wait? @birschick-bq

@Jens-G
Sorry if it wasn't clear. I've replace this PR with a fully spec-driven AI solution here:
#4000

Any one of these three are working versions. The latest is probably the one to go with. But I'm open.