Repository navigation
Commit b8f745a
authored
xds: Fix TSAN data race on ClientCall cancellation in ExternalProcessorClientInterceptor (grpc#13022)
Prevent concurrent cancellations of the underlying ClientCall in
ExternalProcessorClientInterceptor:
- Wrap rawCall with SimpleForwardingClientCall using an AtomicBoolean to
ensure the underlying ClientCall.cancel() is executed at most once, even
if invoked concurrently across threads or from DelayedListener.
- In DataPlaneClientCall.cancel(), atomically transition
extProcStreamState to FAILED, catch exceptions during onError(), and
clear extProcClientCallRequestObserver.
- In sendToExtProc(), return early if the ext-proc stream is already
completed or the observer is null, and catch unexpected onNext()
exceptions to trigger internalOnError() rather than letting exceptions
escape into listener callbacks.
- Safely complete and clear extProcClientCallRequestObserver in
closeExtProcStream() and halfCloseExtProcStream().
- Route all rawCall.cancel() calls in sendMessage(),
handleImmediateResponse(), and DataPlaneListener through
cancelDownstream().
Fixes the TSAN detected race condition below. The previous fix in commit
cf92f2d introduced `AtomicBoolean downstreamCancelled` in
`cancelDownstream()`. However, the TSAN data race still reproduced under
postsubmit because `DelayedListener` exception path bypasses
`cancelDownstream()`: When `DelayedClientCall.start(wrappedListener,
headers)` is called, it registers an internal `DelayedListener`. If
`wrappedListener.onHeaders()` throws any exception,
`DelayedListener.deliverHeaders()` catches it and calls
`exceptionThrown()`, which invokes `delayedCall.cancel()` directly. Once
`delayedCall` has been activated `(realCall != null)`,
`DelayedClientCall.cancel()` directly calls `realCall.cancel()` on
whatever thread triggered it without any de-duplication.
```
Details
==================
WARNING: ThreadSanitizer: data race (pid=7774)
Read of size 1 at 0x000089d8a086 by thread T113:
#0 io.grpc.internal.ClientCallImpl.cancelInternal(Ljava/lang/String;Ljava/lang/Throwable;)V ClientCallImpl.java:464
#1 io.grpc.internal.ClientCallImpl.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ClientCallImpl.java:455
#2 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
#3 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#4 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#5 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
grpc#6 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#7 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#8 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
grpc#9 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#10 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#11 io.grpc.internal.DelayedClientCall$2.run()V DelayedClientCall.java:269
grpc#12 io.grpc.internal.DelayedClientCall.delayOrExecute(Ljava/lang/Runnable;)V DelayedClientCall.java:292
grpc#13 io.grpc.internal.DelayedClientCall.cancel(Lio/grpc/Status;Z)V DelayedClientCall.java:266
grpc#14 io.grpc.internal.DelayedClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V DelayedClientCall.java:242
grpc#15 io.grpc.xds.ExternalProcessorClientInterceptor$DataPlaneClientCall.internalOnError(Ljava/lang/Throwable;)V ExternalProcessorClientInterceptor.java:715
grpc#16 io.grpc.xds.ExternalProcessorClientInterceptor$DataPlaneClientCall$1.onNext(Lio/envoyproxy/envoy/service/ext_proc/v3/ProcessingResponse;)V ExternalProcessorClientInterceptor.java:457
grpc#17 io.grpc.xds.ExternalProcessorClientInterceptor$DataPlaneClientCall$1.onNext(Ljava/lang/Object;)V ExternalProcessorClientInterceptor.java:426
grpc#18 io.grpc.stub.ClientCalls$StreamObserverToCallListenerAdapter.onMessage(Ljava/lang/Object;)V ClientCalls.java:568
grpc#19 io.grpc.ForwardingClientCallListener.onMessage(Ljava/lang/Object;)V ForwardingClientCallListener.java:33
grpc#20 io.grpc.ForwardingClientCallListener.onMessage(Ljava/lang/Object;)V ForwardingClientCallListener.java:33
grpc#21 io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessagesAvailable.runInternal()V ClientCallImpl.java:662
grpc#22 io.grpc.internal.ClientCallImpl$ClientStreamListenerImpl$1MessagesAvailable.runInContext()V ClientCallImpl.java:649
grpc#23 io.grpc.internal.ContextRunnable.run()V ContextRunnable.java:37
grpc#24 io.grpc.internal.SerializingExecutor.run()V SerializingExecutor.java:133
grpc#25 io.grpc.internal.SerializingExecutor.run()V SerializingExecutor.java:133
grpc#26 java.util.concurrent.ThreadPoolExecutor.runWorker(Ljava/util/concurrent/ThreadPoolExecutor$Worker;)V ThreadPoolExecutor.java:1090
grpc#27 java.util.concurrent.ThreadPoolExecutor$Worker.run()V ThreadPoolExecutor.java:614
grpc#28 java.lang.Thread.runWith(Ljava/lang/Object;Ljava/lang/Runnable;)V Thread.java:1560
grpc#29 java.lang.Thread.run()V Thread.java:1547
grpc#30 (Generated Stub) <null>
Previous write of size 1 at 0x000089d8a086 by thread T29:
#0 io.grpc.internal.ClientCallImpl.cancelInternal(Ljava/lang/String;Ljava/lang/Throwable;)V ClientCallImpl.java:467
#1 io.grpc.internal.ClientCallImpl.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ClientCallImpl.java:455
#2 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
#3 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#4 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#5 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
grpc#6 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#7 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#8 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
grpc#9 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#10 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#11 io.grpc.internal.DelayedClientCall$2.run()V DelayedClientCall.java:269
grpc#12 io.grpc.internal.DelayedClientCall.delayOrExecute(Ljava/lang/Runnable;)V DelayedClientCall.java:292
grpc#13 io.grpc.internal.DelayedClientCall.cancel(Lio/grpc/Status;Z)V DelayedClientCall.java:266
grpc#14 io.grpc.internal.DelayedClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V DelayedClientCall.java:242
grpc#15 io.grpc.PartialForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V PartialForwardingClientCall.java:39
grpc#16 io.grpc.ForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:22
grpc#17 io.grpc.ForwardingClientCall$SimpleForwardingClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ForwardingClientCall.java:44
grpc#18 io.grpc.xds.ExternalProcessorClientInterceptor$DataPlaneClientCall.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V ExternalProcessorClientInterceptor.java:887
grpc#19 io.grpc.xds.XdsNameResolver$RawMessageClientInterceptor$2.cancel(Ljava/lang/String;Ljava/lang/Throwable;)V XdsNameResolver.java:1182
grpc#20 io.grpc.xds.ExternalProcessorClientInterceptorTest.clientInterceptor_contextPropagatedToStartCall()V ExternalProcessorClientInterceptorTest.java:13411
```1 parent 64daddc commit b8f745a
2 files changed
Lines changed: 201 additions & 33 deletions
File tree
- xds/src
- main/java/io/grpc/xds
- test/java/io/grpc/xds
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
236 | 236 | | |
237 | 237 | | |
238 | 238 | | |
239 | | - | |
240 | | - | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
241 | 251 | | |
242 | 252 | | |
243 | 253 | | |
| |||
362 | 372 | | |
363 | 373 | | |
364 | 374 | | |
365 | | - | |
366 | 375 | | |
367 | 376 | | |
368 | 377 | | |
| |||
443 | 452 | | |
444 | 453 | | |
445 | 454 | | |
446 | | - | |
447 | | - | |
448 | | - | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
449 | 460 | | |
450 | 461 | | |
451 | 462 | | |
452 | | - | |
453 | 463 | | |
454 | 464 | | |
455 | 465 | | |
| |||
463 | 473 | | |
464 | 474 | | |
465 | 475 | | |
466 | | - | |
| 476 | + | |
467 | 477 | | |
468 | 478 | | |
469 | 479 | | |
| |||
664 | 674 | | |
665 | 675 | | |
666 | 676 | | |
| 677 | + | |
| 678 | + | |
| 679 | + | |
667 | 680 | | |
668 | 681 | | |
669 | 682 | | |
| |||
693 | 706 | | |
694 | 707 | | |
695 | 708 | | |
696 | | - | |
| 709 | + | |
697 | 710 | | |
698 | 711 | | |
699 | 712 | | |
| |||
830 | 843 | | |
831 | 844 | | |
832 | 845 | | |
| 846 | + | |
833 | 847 | | |
834 | 848 | | |
835 | 849 | | |
| |||
839 | 853 | | |
840 | 854 | | |
841 | 855 | | |
842 | | - | |
843 | | - | |
844 | | - | |
845 | | - | |
846 | | - | |
| 856 | + | |
847 | 857 | | |
848 | 858 | | |
849 | 859 | | |
| |||
999 | 1009 | | |
1000 | 1010 | | |
1001 | 1011 | | |
1002 | | - | |
| 1012 | + | |
1003 | 1013 | | |
1004 | 1014 | | |
1005 | 1015 | | |
| |||
1029 | 1039 | | |
1030 | 1040 | | |
1031 | 1041 | | |
1032 | | - | |
| 1042 | + | |
1033 | 1043 | | |
1034 | 1044 | | |
1035 | 1045 | | |
| |||
1131 | 1141 | | |
1132 | 1142 | | |
1133 | 1143 | | |
1134 | | - | |
1135 | | - | |
1136 | | - | |
1137 | | - | |
| 1144 | + | |
| 1145 | + | |
1138 | 1146 | | |
1139 | 1147 | | |
1140 | 1148 | | |
1141 | 1149 | | |
1142 | 1150 | | |
1143 | | - | |
1144 | | - | |
1145 | | - | |
1146 | | - | |
1147 | | - | |
1148 | | - | |
| 1151 | + | |
| 1152 | + | |
| 1153 | + | |
| 1154 | + | |
| 1155 | + | |
| 1156 | + | |
| 1157 | + | |
| 1158 | + | |
| 1159 | + | |
1149 | 1160 | | |
1150 | 1161 | | |
1151 | 1162 | | |
| |||
1318 | 1329 | | |
1319 | 1330 | | |
1320 | 1331 | | |
1321 | | - | |
| 1332 | + | |
1322 | 1333 | | |
1323 | 1334 | | |
1324 | 1335 | | |
| |||
1442 | 1453 | | |
1443 | 1454 | | |
1444 | 1455 | | |
1445 | | - | |
1446 | 1456 | | |
1447 | 1457 | | |
1448 | 1458 | | |
| |||
1457 | 1467 | | |
1458 | 1468 | | |
1459 | 1469 | | |
1460 | | - | |
1461 | 1470 | | |
1462 | 1471 | | |
1463 | | - | |
1464 | 1472 | | |
1465 | 1473 | | |
1466 | 1474 | | |
| |||
1538 | 1546 | | |
1539 | 1547 | | |
1540 | 1548 | | |
1541 | | - | |
| 1549 | + | |
1542 | 1550 | | |
1543 | 1551 | | |
1544 | 1552 | | |
| |||
1574 | 1582 | | |
1575 | 1583 | | |
1576 | 1584 | | |
1577 | | - | |
| 1585 | + | |
1578 | 1586 | | |
1579 | 1587 | | |
1580 | 1588 | | |
| |||
1592 | 1600 | | |
1593 | 1601 | | |
1594 | 1602 | | |
1595 | | - | |
| 1603 | + | |
1596 | 1604 | | |
1597 | 1605 | | |
1598 | 1606 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
| 93 | + | |
93 | 94 | | |
94 | 95 | | |
95 | 96 | | |
| 97 | + | |
96 | 98 | | |
97 | 99 | | |
98 | 100 | | |
| |||
21129 | 21131 | | |
21130 | 21132 | | |
21131 | 21133 | | |
| 21134 | + | |
| 21135 | + | |
| 21136 | + | |
| 21137 | + | |
| 21138 | + | |
| 21139 | + | |
| 21140 | + | |
| 21141 | + | |
| 21142 | + | |
| 21143 | + | |
| 21144 | + | |
| 21145 | + | |
| 21146 | + | |
| 21147 | + | |
| 21148 | + | |
| 21149 | + | |
| 21150 | + | |
| 21151 | + | |
| 21152 | + | |
| 21153 | + | |
| 21154 | + | |
| 21155 | + | |
| 21156 | + | |
| 21157 | + | |
| 21158 | + | |
| 21159 | + | |
| 21160 | + | |
| 21161 | + | |
| 21162 | + | |
| 21163 | + | |
| 21164 | + | |
| 21165 | + | |
| 21166 | + | |
| 21167 | + | |
| 21168 | + | |
| 21169 | + | |
| 21170 | + | |
| 21171 | + | |
| 21172 | + | |
| 21173 | + | |
| 21174 | + | |
| 21175 | + | |
| 21176 | + | |
| 21177 | + | |
| 21178 | + | |
| 21179 | + | |
| 21180 | + | |
| 21181 | + | |
| 21182 | + | |
| 21183 | + | |
| 21184 | + | |
| 21185 | + | |
| 21186 | + | |
| 21187 | + | |
| 21188 | + | |
| 21189 | + | |
| 21190 | + | |
| 21191 | + | |
| 21192 | + | |
| 21193 | + | |
| 21194 | + | |
| 21195 | + | |
| 21196 | + | |
| 21197 | + | |
| 21198 | + | |
| 21199 | + | |
| 21200 | + | |
| 21201 | + | |
| 21202 | + | |
| 21203 | + | |
| 21204 | + | |
| 21205 | + | |
| 21206 | + | |
| 21207 | + | |
| 21208 | + | |
| 21209 | + | |
| 21210 | + | |
| 21211 | + | |
| 21212 | + | |
| 21213 | + | |
| 21214 | + | |
| 21215 | + | |
| 21216 | + | |
| 21217 | + | |
| 21218 | + | |
| 21219 | + | |
| 21220 | + | |
| 21221 | + | |
| 21222 | + | |
| 21223 | + | |
| 21224 | + | |
| 21225 | + | |
| 21226 | + | |
| 21227 | + | |
| 21228 | + | |
| 21229 | + | |
| 21230 | + | |
| 21231 | + | |
| 21232 | + | |
| 21233 | + | |
| 21234 | + | |
| 21235 | + | |
| 21236 | + | |
| 21237 | + | |
| 21238 | + | |
| 21239 | + | |
| 21240 | + | |
| 21241 | + | |
| 21242 | + | |
| 21243 | + | |
| 21244 | + | |
| 21245 | + | |
| 21246 | + | |
| 21247 | + | |
| 21248 | + | |
| 21249 | + | |
| 21250 | + | |
| 21251 | + | |
| 21252 | + | |
| 21253 | + | |
| 21254 | + | |
| 21255 | + | |
| 21256 | + | |
| 21257 | + | |
| 21258 | + | |
| 21259 | + | |
| 21260 | + | |
| 21261 | + | |
| 21262 | + | |
| 21263 | + | |
| 21264 | + | |
| 21265 | + | |
| 21266 | + | |
| 21267 | + | |
| 21268 | + | |
| 21269 | + | |
| 21270 | + | |
| 21271 | + | |
| 21272 | + | |
| 21273 | + | |
| 21274 | + | |
| 21275 | + | |
| 21276 | + | |
| 21277 | + | |
| 21278 | + | |
| 21279 | + | |
| 21280 | + | |
| 21281 | + | |
| 21282 | + | |
| 21283 | + | |
| 21284 | + | |
| 21285 | + | |
| 21286 | + | |
| 21287 | + | |
| 21288 | + | |
| 21289 | + | |
| 21290 | + | |
| 21291 | + | |
21132 | 21292 | | |
21133 | 21293 | | |
21134 | 21294 | | |
| |||
0 commit comments