aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
Diffstat
-rw-r--r--CMakeLists.txt33
-rw-r--r--Makefile36
-rw-r--r--build.yaml15
-rw-r--r--src/core/lib/iomgr/ev_epollex_linux.cc30
-rw-r--r--test/core/iomgr/BUILD21
-rw-r--r--test/core/iomgr/ev_epollex_linux_test.cc115
-rw-r--r--tools/run_tests/generated/sources_and_headers.json17
-rw-r--r--tools/run_tests/generated/tests.json20
8 files changed, 273 insertions, 14 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 9f532b047d..ff46d9cfcc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -244,6 +244,9 @@ endif()
add_dependencies(buildtests_c endpoint_pair_test)
add_dependencies(buildtests_c error_test)
if(_gRPC_PLATFORM_LINUX)
+add_dependencies(buildtests_c ev_epollex_linux_test)
+endif()
+if(_gRPC_PLATFORM_LINUX)
add_dependencies(buildtests_c ev_epollsig_linux_test)
endif()
add_dependencies(buildtests_c fake_resolver_test)
@@ -6163,6 +6166,36 @@ endif (gRPC_BUILD_TESTS)
if (gRPC_BUILD_TESTS)
if(_gRPC_PLATFORM_LINUX)
+add_executable(ev_epollex_linux_test
+ test/core/iomgr/ev_epollex_linux_test.cc
+)
+
+
+target_include_directories(ev_epollex_linux_test
+ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
+ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include
+ PRIVATE ${_gRPC_SSL_INCLUDE_DIR}
+ PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR}
+ PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR}
+ PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR}
+ PRIVATE ${_gRPC_CARES_INCLUDE_DIR}
+ PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR}
+ PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
+)
+
+target_link_libraries(ev_epollex_linux_test
+ ${_gRPC_ALLTARGETS_LIBRARIES}
+ grpc_test_util
+ grpc
+ gpr_test_util
+ gpr
+)
+
+endif()
+endif (gRPC_BUILD_TESTS)
+if (gRPC_BUILD_TESTS)
+if(_gRPC_PLATFORM_LINUX)
+
add_executable(ev_epollsig_linux_test
test/core/iomgr/ev_epollsig_linux_test.cc
)
diff --git a/Makefile b/Makefile
index bc51761d36..dd194dc006 100644
--- a/Makefile
+++ b/Makefile
@@ -970,6 +970,7 @@ dns_resolver_test: $(BINDIR)/$(CONFIG)/dns_resolver_test
dualstack_socket_test: $(BINDIR)/$(CONFIG)/dualstack_socket_test
endpoint_pair_test: $(BINDIR)/$(CONFIG)/endpoint_pair_test
error_test: $(BINDIR)/$(CONFIG)/error_test
+ev_epollex_linux_test: $(BINDIR)/$(CONFIG)/ev_epollex_linux_test
ev_epollsig_linux_test: $(BINDIR)/$(CONFIG)/ev_epollsig_linux_test
fake_resolver_test: $(BINDIR)/$(CONFIG)/fake_resolver_test
fake_transport_security_test: $(BINDIR)/$(CONFIG)/fake_transport_security_test
@@ -1415,6 +1416,7 @@ buildtests_c: privatelibs_c \
$(BINDIR)/$(CONFIG)/dualstack_socket_test \
$(BINDIR)/$(CONFIG)/endpoint_pair_test \
$(BINDIR)/$(CONFIG)/error_test \
+ $(BINDIR)/$(CONFIG)/ev_epollex_linux_test \
$(BINDIR)/$(CONFIG)/ev_epollsig_linux_test \
$(BINDIR)/$(CONFIG)/fake_resolver_test \
$(BINDIR)/$(CONFIG)/fake_transport_security_test \
@@ -1935,6 +1937,8 @@ test_c: buildtests_c
$(Q) $(BINDIR)/$(CONFIG)/endpoint_pair_test || ( echo test endpoint_pair_test failed ; exit 1 )
$(E) "[RUN] Testing error_test"
$(Q) $(BINDIR)/$(CONFIG)/error_test || ( echo test error_test failed ; exit 1 )
+ $(E) "[RUN] Testing ev_epollex_linux_test"
+ $(Q) $(BINDIR)/$(CONFIG)/ev_epollex_linux_test || ( echo test ev_epollex_linux_test failed ; exit 1 )
$(E) "[RUN] Testing ev_epollsig_linux_test"
$(Q) $(BINDIR)/$(CONFIG)/ev_epollsig_linux_test || ( echo test ev_epollsig_linux_test failed ; exit 1 )
$(E) "[RUN] Testing fake_resolver_test"
@@ -11046,6 +11050,38 @@ endif
endif
+EV_EPOLLEX_LINUX_TEST_SRC = \
+ test/core/iomgr/ev_epollex_linux_test.cc \
+
+EV_EPOLLEX_LINUX_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(EV_EPOLLEX_LINUX_TEST_SRC))))
+ifeq ($(NO_SECURE),true)
+
+# You can't build secure targets if you don't have OpenSSL.
+
+$(BINDIR)/$(CONFIG)/ev_epollex_linux_test: openssl_dep_error
+
+else
+
+
+
+$(BINDIR)/$(CONFIG)/ev_epollex_linux_test: $(EV_EPOLLEX_LINUX_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a
+ $(E) "[LD] Linking $@"
+ $(Q) mkdir -p `dirname $@`
+ $(Q) $(LD) $(LDFLAGS) $(EV_EPOLLEX_LINUX_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/ev_epollex_linux_test
+
+endif
+
+$(OBJDIR)/$(CONFIG)/test/core/iomgr/ev_epollex_linux_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a
+
+deps_ev_epollex_linux_test: $(EV_EPOLLEX_LINUX_TEST_OBJS:.o=.dep)
+
+ifneq ($(NO_SECURE),true)
+ifneq ($(NO_DEPS),true)
+-include $(EV_EPOLLEX_LINUX_TEST_OBJS:.o=.dep)
+endif
+endif
+
+
EV_EPOLLSIG_LINUX_TEST_SRC = \
test/core/iomgr/ev_epollsig_linux_test.cc \
diff --git a/build.yaml b/build.yaml
index e8620bfb4c..6a4304116c 100644
--- a/build.yaml
+++ b/build.yaml
@@ -2277,6 +2277,21 @@ targets:
- gpr_test_util
- gpr
uses_polling: false
+- name: ev_epollex_linux_test
+ cpu_cost: 3
+ build: test
+ language: c
+ src:
+ - test/core/iomgr/ev_epollex_linux_test.cc
+ deps:
+ - grpc_test_util
+ - grpc
+ - gpr_test_util
+ - gpr
+ exclude_iomgrs:
+ - uv
+ platforms:
+ - linux
- name: ev_epollsig_linux_test
cpu_cost: 3
build: test
diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc
index f3db72e85c..1ce4181143 100644
--- a/src/core/lib/iomgr/ev_epollex_linux.cc
+++ b/src/core/lib/iomgr/ev_epollex_linux.cc
@@ -339,21 +339,29 @@ static void ref_by(grpc_fd* fd, int n) {
GPR_ASSERT(gpr_atm_no_barrier_fetch_add(&fd->refst, n) > 0);
}
+/* Uninitialize and add to the freelist */
static void fd_destroy(void* arg, grpc_error* error) {
grpc_fd* fd = static_cast<grpc_fd*>(arg);
- /* Add the fd to the freelist */
grpc_iomgr_unregister_object(&fd->iomgr_object);
POLLABLE_UNREF(fd->pollable_obj, "fd_pollable");
gpr_mu_destroy(&fd->pollable_mu);
gpr_mu_destroy(&fd->orphan_mu);
- gpr_mu_lock(&fd_freelist_mu);
- fd->freelist_next = fd_freelist;
- fd_freelist = fd;
fd->read_closure->DestroyEvent();
fd->write_closure->DestroyEvent();
fd->error_closure->DestroyEvent();
+#ifndef NDEBUG
+ // Since we do not call gpr_free on the fd (and put it in a freelist instead),
+ // the following will help us catch any 'use-after-fd_destroy()' errors in the
+ // code
+ memset(fd, 0xFF, sizeof(grpc_fd));
+#endif
+
+ /* Add the fd to the freelist */
+ gpr_mu_lock(&fd_freelist_mu);
+ fd->freelist_next = fd_freelist;
+ fd_freelist = fd;
gpr_mu_unlock(&fd_freelist_mu);
}
@@ -409,20 +417,18 @@ static grpc_fd* fd_create(int fd, const char* name, bool track_err) {
new_fd->error_closure.Init();
}
- gpr_mu_init(&new_fd->pollable_mu);
- gpr_mu_init(&new_fd->orphan_mu);
- new_fd->pollable_obj = nullptr;
- gpr_atm_rel_store(&new_fd->refst, (gpr_atm)1);
new_fd->fd = fd;
- new_fd->track_err = track_err;
new_fd->salt = gpr_atm_no_barrier_fetch_add(&g_fd_salt, 1);
+ gpr_atm_rel_store(&new_fd->refst, (gpr_atm)1);
+ gpr_mu_init(&new_fd->orphan_mu);
+ gpr_mu_init(&new_fd->pollable_mu);
+ new_fd->pollable_obj = nullptr;
new_fd->read_closure->InitEvent();
new_fd->write_closure->InitEvent();
new_fd->error_closure->InitEvent();
- gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL);
-
new_fd->freelist_next = nullptr;
new_fd->on_done_closure = nullptr;
+ gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL);
char* fd_name;
gpr_asprintf(&fd_name, "%s fd=%d", name, fd);
@@ -433,6 +439,8 @@ static grpc_fd* fd_create(int fd, const char* name, bool track_err) {
}
#endif
gpr_free(fd_name);
+
+ new_fd->track_err = track_err;
return new_fd;
}
diff --git a/test/core/iomgr/BUILD b/test/core/iomgr/BUILD
index bbf0815e6f..cc1b6aee5e 100644
--- a/test/core/iomgr/BUILD
+++ b/test/core/iomgr/BUILD
@@ -18,7 +18,10 @@ licenses(["notice"]) # Apache v2
load("//test/core/util:grpc_fuzzer.bzl", "grpc_fuzzer")
-grpc_package(name = "test/core/iomgr", visibility = "public") # Useful for third party devs to test their io manager implementation.
+grpc_package(
+ name = "test/core/iomgr",
+ visibility = "public",
+) # Useful for third party devs to test their io manager implementation.
grpc_cc_library(
name = "endpoint_tests",
@@ -73,15 +76,27 @@ grpc_cc_test(
)
grpc_cc_test(
+ name = "ev_epollex_linux_test",
+ srcs = ["ev_epollex_linux_test.cc"],
+ language = "C++",
+ deps = [
+ "//:gpr",
+ "//:grpc",
+ "//test/core/util:gpr_test_util",
+ "//test/core/util:grpc_test_util",
+ ],
+)
+
+grpc_cc_test(
name = "ev_epollsig_linux_test",
srcs = ["ev_epollsig_linux_test.cc"],
+ language = "C++",
deps = [
"//:gpr",
"//:grpc",
"//test/core/util:gpr_test_util",
"//test/core/util:grpc_test_util",
],
- language = "C++",
)
grpc_cc_test(
@@ -221,13 +236,13 @@ grpc_cc_test(
name = "tcp_server_posix_test",
srcs = ["tcp_server_posix_test.cc"],
language = "C++",
+ tags = ["manual"], # TODO(adelez): Remove once this works on Foundry.
deps = [
"//:gpr",
"//:grpc",
"//test/core/util:gpr_test_util",
"//test/core/util:grpc_test_util",
],
- tags = ["manual"], # TODO(adelez): Remove once this works on Foundry.
)
grpc_cc_test(
diff --git a/test/core/iomgr/ev_epollex_linux_test.cc b/test/core/iomgr/ev_epollex_linux_test.cc
new file mode 100644
index 0000000000..08d1e68b39
--- /dev/null
+++ b/test/core/iomgr/ev_epollex_linux_test.cc
@@ -0,0 +1,115 @@
+/*
+ *
+ * Copyright 2018 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+#include "src/core/lib/iomgr/port.h"
+
+/* This test only relevant on linux systems where epoll() is available */
+#if defined(GRPC_LINUX_EPOLL_CREATE1) && defined(GRPC_LINUX_EVENTFD)
+#include "src/core/lib/iomgr/ev_epollex_linux.h"
+
+#include <grpc/grpc.h>
+#include <string.h>
+#include <sys/eventfd.h>
+
+#include "test/core/util/test_config.h"
+
+static void pollset_destroy(void* ps, grpc_error* error) {
+ grpc_pollset_destroy(static_cast<grpc_pollset*>(ps));
+ gpr_free(ps);
+}
+
+// This test is added to cover the case found in bug:
+// https://github.com/grpc/grpc/issues/15760
+static void test_pollable_owner_fd() {
+ grpc_core::ExecCtx exec_ctx;
+ int ev_fd1;
+ int ev_fd2;
+ grpc_fd* grpc_fd1;
+ grpc_fd* grpc_fd2;
+ grpc_pollset* ps;
+ gpr_mu* mu;
+
+ // == Create two grpc_fds ==
+ // All we need is two file descriptors. Doesn't matter what type. We use
+ // eventfd type here for the purpose of this test
+ ev_fd1 = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+ ev_fd2 = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+ if (ev_fd1 < 0 || ev_fd2 < 0) {
+ gpr_log(GPR_ERROR, "Error in creating event fds for the test");
+ return;
+ }
+ grpc_fd1 = grpc_fd_create(ev_fd1, "epollex-test-fd1", false);
+ grpc_fd2 = grpc_fd_create(ev_fd2, "epollex-test-fd2", false);
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // == Create a pollset ==
+ ps = static_cast<grpc_pollset*>(gpr_zalloc(grpc_pollset_size()));
+ grpc_pollset_init(ps, &mu);
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // == Add fd1 to pollset ==
+ grpc_pollset_add_fd(ps, grpc_fd1);
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // == Destroy fd1 ==
+ grpc_fd_orphan(grpc_fd1, nullptr, nullptr, "test fd1 orphan");
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // = Add fd2 to pollset ==
+ //
+ // Before https://github.com/grpc/grpc/issues/15760, the following line caused
+ // unexpected behavior (The previous grpc_pollset_add_fd(ps, grpc_fd1) created
+ // an underlying structure in epollex that held a reference to grpc_fd1 which
+ // was being accessed here even after grpc_fd_orphan(grpc_fd1) was called
+ grpc_pollset_add_fd(ps, grpc_fd2);
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // == Destroy fd2 ==
+ grpc_fd_orphan(grpc_fd2, nullptr, nullptr, "test fd2 orphan");
+ grpc_core::ExecCtx::Get()->Flush();
+
+ // == Destroy pollset
+ grpc_closure ps_destroy_closure;
+ GRPC_CLOSURE_INIT(&ps_destroy_closure, pollset_destroy, ps,
+ grpc_schedule_on_exec_ctx);
+ grpc_pollset_shutdown(ps, &ps_destroy_closure);
+ grpc_core::ExecCtx::Get()->Flush();
+}
+
+int main(int argc, char** argv) {
+ const char* poll_strategy = nullptr;
+ grpc_test_init(argc, argv);
+ grpc_init();
+ {
+ grpc_core::ExecCtx exec_ctx;
+ poll_strategy = grpc_get_poll_strategy_name();
+ if (poll_strategy != nullptr && strcmp(poll_strategy, "epollex") == 0) {
+ test_pollable_owner_fd();
+ } else {
+ gpr_log(GPR_INFO,
+ "Skipping the test. The test is only relevant for 'epollex' "
+ "strategy. and the current strategy is: '%s'",
+ poll_strategy);
+ }
+ }
+
+ grpc_shutdown();
+ return 0;
+}
+#else /* defined(GRPC_LINUX_EPOLL_CREATE1) && defined(GRPC_LINUX_EVENTFD) */
+int main(int argc, char** argv) { return 0; }
+#endif
diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json
index 4b5f7d51ea..0162de8217 100644
--- a/tools/run_tests/generated/sources_and_headers.json
+++ b/tools/run_tests/generated/sources_and_headers.json
@@ -459,6 +459,23 @@
"headers": [],
"is_filegroup": false,
"language": "c",
+ "name": "ev_epollex_linux_test",
+ "src": [
+ "test/core/iomgr/ev_epollex_linux_test.cc"
+ ],
+ "third_party": false,
+ "type": "target"
+ },
+ {
+ "deps": [
+ "gpr",
+ "gpr_test_util",
+ "grpc",
+ "grpc_test_util"
+ ],
+ "headers": [],
+ "is_filegroup": false,
+ "language": "c",
"name": "ev_epollsig_linux_test",
"src": [
"test/core/iomgr/ev_epollsig_linux_test.cc"
diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json
index ba63352036..afd654bf33 100644
--- a/tools/run_tests/generated/tests.json
+++ b/tools/run_tests/generated/tests.json
@@ -575,6 +575,26 @@
"flaky": false,
"gtest": false,
"language": "c",
+ "name": "ev_epollex_linux_test",
+ "platforms": [
+ "linux"
+ ],
+ "uses_polling": true
+ },
+ {
+ "args": [],
+ "benchmark": false,
+ "ci_platforms": [
+ "linux"
+ ],
+ "cpu_cost": 3,
+ "exclude_configs": [],
+ "exclude_iomgrs": [
+ "uv"
+ ],
+ "flaky": false,
+ "gtest": false,
+ "language": "c",
"name": "ev_epollsig_linux_test",
"platforms": [
"linux"
generated by cgit v1.2.3 (git 2.39.1) at 2024-11-30 11:48:53 +0000