Skip to content

Commit 1a280e9

Browse files
committed
Fix thread-safety in DelayedClientTransport and remove Thread.sleep in E2E delay tests
1 parent a7cceb4 commit 1a280e9

3 files changed

Lines changed: 6 additions & 6 deletions

File tree

core/src/main/java/io/grpc/internal/DelayedClientTransport.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,9 @@ private class PendingStream extends DelayedStream {
401401
private final Context context = Context.current();
402402
private final ClientStreamTracer[] tracers;
403403
private volatile Status lastPickStatus;
404+
@GuardedBy("this")
404405
@Nullable private String activeDelayType;
406+
@GuardedBy("this")
405407
@Nullable private String activeDelayReason;
406408

407409
private PendingStream(PickSubchannelArgs args, ClientStreamTracer[] tracers,
@@ -425,7 +427,7 @@ private PendingStream(PickSubchannelArgs args, ClientStreamTracer[] tracers,
425427
* spans are ended and a new segment is initiated. If only {@code newReason} changes, a
426428
* structured transition event is appended to the active span without span re-creation.
427429
*/
428-
void updateDelay(@Nullable String newType, @Nullable String newReason) {
430+
synchronized void updateDelay(@Nullable String newType, @Nullable String newReason) {
429431
if (!Objects.equals(activeDelayType, newType)) {
430432
// Delay type changed (e.g., from RLS lookup to connecting). End the previous delay.
431433
if (activeDelayType != null) {
@@ -453,7 +455,7 @@ void updateDelay(@Nullable String newType, @Nullable String newReason) {
453455
/**
454456
* Ends active attempt delay segment telemetry upon stream creation or stream cancellation.
455457
*/
456-
void endDelay() {
458+
synchronized void endDelay() {
457459
if (activeDelayType != null) {
458460
for (ClientStreamTracer tracer : tracers) {
459461
tracer.recordAttemptDelayEnd();

opentelemetry/src/test/java/io/grpc/opentelemetry/OpenTelemetryMetricsModuleTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1690,11 +1690,11 @@ public Status acceptResolvedAddresses(LoadBalancer.ResolvedAddresses resolvedAdd
16901690
helper.updateBalancingState(ConnectivityState.CONNECTING, new SubchannelPicker() {
16911691
@Override
16921692
public PickResult pickSubchannel(PickSubchannelArgs args) {
1693+
latch.countDown();
16931694
return PickResult.withNoResult("connecting",
16941695
"Simulated slow TLS handshake with backend");
16951696
}
16961697
});
1697-
latch.countDown();
16981698
return Status.OK;
16991699
}
17001700

@@ -1769,7 +1769,6 @@ public void shutdown() {}
17691769
call.request(1);
17701770

17711771
latch.await(5, TimeUnit.SECONDS);
1772-
Thread.sleep(50);
17731772
call.cancel("End test delay segment", null);
17741773
} finally {
17751774
channel.shutdownNow();

opentelemetry/src/test/java/io/grpc/opentelemetry/OpenTelemetryTracingModuleTest.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -481,11 +481,11 @@ public Status acceptResolvedAddresses(LoadBalancer.ResolvedAddresses resolvedAdd
481481
helper.updateBalancingState(ConnectivityState.CONNECTING, new SubchannelPicker() {
482482
@Override
483483
public PickResult pickSubchannel(PickSubchannelArgs args) {
484+
latch.countDown();
484485
return PickResult.withNoResult("connecting",
485486
"Simulated slow TLS handshake with backend");
486487
}
487488
});
488-
latch.countDown();
489489
return Status.OK;
490490
}
491491

@@ -559,7 +559,6 @@ public void shutdown() {}
559559
call.request(1);
560560

561561
latch.await(5, TimeUnit.SECONDS);
562-
Thread.sleep(50);
563562
call.cancel("End test delay segment", null);
564563
} finally {
565564
channel.shutdownNow();

0 commit comments

Comments
 (0)