Merge pull request #13294 from sreecha/cc-tsan-1
Fix TSAN issue in backup poller
diff --git a/build.yaml b/build.yaml
index 4f43125..cd6486e 100644
--- a/build.yaml
+++ b/build.yaml
@@ -3475,8 +3475,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3498,8 +3496,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3521,8 +3517,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3544,8 +3538,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3566,8 +3558,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3588,8 +3578,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3610,8 +3598,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=4
benchmark: true
defaults: benchmark
platforms:
@@ -3632,8 +3618,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3657,8 +3641,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
excluded_poll_engines:
@@ -3685,8 +3667,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
excluded_poll_engines:
@@ -3712,8 +3692,6 @@
- gpr_test_util
- gpr
- grpc++_test_config
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
exclude_configs:
@@ -3742,8 +3720,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
excluded_poll_engines:
@@ -3768,8 +3744,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
@@ -3791,8 +3765,6 @@
- grpc_unsecure
- gpr_test_util
- gpr
- args:
- - --benchmark_min_time=0
benchmark: true
defaults: benchmark
platforms:
diff --git a/doc/connectivity-semantics-and-api.md b/doc/connectivity-semantics-and-api.md
index 6d39619..dc30fe5 100644
--- a/doc/connectivity-semantics-and-api.md
+++ b/doc/connectivity-semantics-and-api.md
@@ -115,8 +115,14 @@
-----------------
All gRPC libraries will expose a channel-level API method to poll the current
-state of a channel. In C++, this method is called GetCurrentState and returns
-an enum for one of the five legal states.
+state of a channel. In C++, this method is called GetState and returns an enum
+for one of the five legal states. It also accepts a boolean `try_to_connect` to
+transition to CONNECTING if the channel is currently IDLE. The boolean should
+act as if an RPC occurred, so it should also reset IDLE_TIMEOUT.
+
+```cpp
+grpc_connectivity_state GetState(bool try_to_connect);
+```
All libraries should also expose an API that enables the application (user of
the gRPC API) to be notified when the channel state changes. Since state
@@ -127,11 +133,11 @@
The synchronous version of this API is:
```cpp
-bool WaitForStateChange(gpr_timespec deadline, ChannelState source_state);
+bool WaitForStateChange(grpc_connectivity_state source_state, gpr_timespec deadline);
```
-which returns true when the state changes to something other than the
-source_state and false if the deadline expires. Asynchronous and futures based
+which returns `true` when the state is something other than the
+`source_state` and `false` if the deadline expires. Asynchronous- and futures-based
APIs should have a corresponding method that allows the application to be
notified when the state of a channel changes.
diff --git a/src/core/lib/iomgr/ev_epoll1_linux.cc b/src/core/lib/iomgr/ev_epoll1_linux.cc
index 9e3643f..61da996 100644
--- a/src/core/lib/iomgr/ev_epoll1_linux.cc
+++ b/src/core/lib/iomgr/ev_epoll1_linux.cc
@@ -18,6 +18,8 @@
#include "src/core/lib/iomgr/port.h"
+#include <grpc/support/log.h>
+
/* This polling engine is only relevant on linux kernels supporting epoll() */
#ifdef GRPC_LINUX_EPOLL
#include "src/core/lib/iomgr/ev_epoll1_linux.h"
@@ -34,7 +36,6 @@
#include <grpc/support/alloc.h>
#include <grpc/support/cpu.h>
-#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <grpc/support/tls.h>
#include <grpc/support/useful.h>
@@ -1230,6 +1231,7 @@
* support is available */
const grpc_event_engine_vtable* grpc_init_epoll1_linux(bool explicit_request) {
if (!grpc_has_wakeup_fd()) {
+ gpr_log(GPR_ERROR, "Skipping epoll1 because of no wakeup fd.");
return NULL;
}
@@ -1254,6 +1256,8 @@
/* If GRPC_LINUX_EPOLL is not defined, it means epoll is not available. Return
* NULL */
const grpc_event_engine_vtable* grpc_init_epoll1_linux(bool explicit_request) {
+ gpr_log(GPR_ERROR,
+ "Skipping epoll1 becuase GRPC_LINUX_EPOLL is not defined.");
return NULL;
}
#endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc
index 26ed1f6..caaee76 100644
--- a/src/core/lib/iomgr/ev_epollex_linux.cc
+++ b/src/core/lib/iomgr/ev_epollex_linux.cc
@@ -18,6 +18,8 @@
#include "src/core/lib/iomgr/port.h"
+#include <grpc/support/log.h>
+
/* This polling engine is only relevant on linux kernels supporting epoll() */
#ifdef GRPC_LINUX_EPOLL
@@ -34,7 +36,6 @@
#include <unistd.h>
#include <grpc/support/alloc.h>
-#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <grpc/support/tls.h>
#include <grpc/support/useful.h>
@@ -1451,10 +1452,12 @@
}
if (!grpc_has_wakeup_fd()) {
+ gpr_log(GPR_ERROR, "Skipping epollex because of no wakeup fd.");
return NULL;
}
if (!grpc_is_epollexclusive_available()) {
+ gpr_log(GPR_INFO, "Skipping epollex because it is not supported.");
return NULL;
}
@@ -1480,6 +1483,8 @@
* NULL */
const grpc_event_engine_vtable* grpc_init_epollex_linux(
bool explicitly_requested) {
+ gpr_log(GPR_ERROR,
+ "Skipping epollex becuase GRPC_LINUX_EPOLL is not defined.");
return NULL;
}
#endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_epollsig_linux.cc b/src/core/lib/iomgr/ev_epollsig_linux.cc
index 9a12780..42806e9 100644
--- a/src/core/lib/iomgr/ev_epollsig_linux.cc
+++ b/src/core/lib/iomgr/ev_epollsig_linux.cc
@@ -19,6 +19,7 @@
#include "src/core/lib/iomgr/port.h"
#include <grpc/grpc_posix.h>
+#include <grpc/support/log.h>
/* This polling engine is only relevant on linux kernels supporting epoll() */
#ifdef GRPC_LINUX_EPOLL
@@ -37,7 +38,6 @@
#include <unistd.h>
#include <grpc/support/alloc.h>
-#include <grpc/support/log.h>
#include <grpc/support/string_util.h>
#include <grpc/support/tls.h>
#include <grpc/support/useful.h>
@@ -1711,14 +1711,17 @@
bool explicit_request) {
/* If use of signals is disabled, we cannot use epoll engine*/
if (is_grpc_wakeup_signal_initialized && grpc_wakeup_signal < 0) {
+ gpr_log(GPR_ERROR, "Skipping epollsig because use of signals is disabled.");
return NULL;
}
if (!grpc_has_wakeup_fd()) {
+ gpr_log(GPR_ERROR, "Skipping epollsig because of no wakeup fd.");
return NULL;
}
if (!is_epoll_available()) {
+ gpr_log(GPR_ERROR, "Skipping epollsig because epoll is unavailable.");
return NULL;
}
@@ -1726,6 +1729,8 @@
if (explicit_request) {
grpc_use_signal(SIGRTMIN + 6);
} else {
+ gpr_log(GPR_ERROR,
+ "Skipping epollsig because uninitialized wakeup signal.");
return NULL;
}
}
@@ -1751,6 +1756,8 @@
* NULL */
const grpc_event_engine_vtable* grpc_init_epollsig_linux(
bool explicit_request) {
+ gpr_log(GPR_ERROR,
+ "Skipping epollsig becuase GRPC_LINUX_EPOLL is not defined.");
return NULL;
}
#endif /* defined(GRPC_POSIX_SOCKET) */
diff --git a/src/core/lib/iomgr/ev_poll_posix.cc b/src/core/lib/iomgr/ev_poll_posix.cc
index 554a438..5745a2a 100644
--- a/src/core/lib/iomgr/ev_poll_posix.cc
+++ b/src/core/lib/iomgr/ev_poll_posix.cc
@@ -1712,6 +1712,7 @@
const grpc_event_engine_vtable* grpc_init_poll_posix(bool explicit_request) {
if (!grpc_has_wakeup_fd()) {
+ gpr_log(GPR_ERROR, "Skipping poll because of no wakeup fd.");
return NULL;
}
if (!GRPC_LOG_IF_ERROR("pollset_global_init", pollset_global_init())) {
diff --git a/src/core/lib/iomgr/ev_posix.cc b/src/core/lib/iomgr/ev_posix.cc
index f72f508..a05279a 100644
--- a/src/core/lib/iomgr/ev_posix.cc
+++ b/src/core/lib/iomgr/ev_posix.cc
@@ -172,12 +172,12 @@
gpr_free(strings[i]);
}
gpr_free(strings);
- gpr_free(s);
if (g_event_engine == NULL) {
- gpr_log(GPR_ERROR, "No event engine could be initialized");
+ gpr_log(GPR_ERROR, "No event engine could be initialized from %s", s);
abort();
}
+ gpr_free(s);
}
void grpc_event_engine_shutdown(void) {
diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json
index 5ba06bf..5df5a74 100644
--- a/tools/run_tests/generated/tests.json
+++ b/tools/run_tests/generated/tests.json
@@ -2964,9 +2964,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -2988,9 +2986,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3012,9 +3008,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3036,9 +3030,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3060,9 +3052,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3084,9 +3074,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3108,9 +3096,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=4"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3132,9 +3118,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3156,9 +3140,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3185,9 +3167,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3214,9 +3194,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3245,9 +3223,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3274,9 +3250,7 @@
"uses_polling": true
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",
@@ -3298,9 +3272,7 @@
"uses_polling": false
},
{
- "args": [
- "--benchmark_min_time=0"
- ],
+ "args": [],
"benchmark": true,
"ci_platforms": [
"linux",