Added a test to catch such things in future

pull/15776/head
Sree Kuchibhotla 7 years ago
parent f5691a5fec
commit 16ad9b8280
  1. 33
      CMakeLists.txt
  2. 36
      Makefile
  3. 15
      build.yaml
  4. 30
      src/core/lib/iomgr/ev_epollex_linux.cc
  5. 21
      test/core/iomgr/BUILD
  6. 115
      test/core/iomgr/ev_epollex_linux_test.cc
  7. 17
      tools/run_tests/generated/sources_and_headers.json
  8. 20
      tools/run_tests/generated/tests.json

@ -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
)

@ -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 \

@ -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

@ -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;
}

@ -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",
@ -72,16 +75,28 @@ 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(

@ -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

@ -449,6 +449,23 @@
"third_party": false,
"type": "target"
},
{
"deps": [
"gpr",
"gpr_test_util",
"grpc",
"grpc_test_util"
],
"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",

@ -561,6 +561,26 @@
],
"uses_polling": false
},
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux"
],
"cpu_cost": 3,
"exclude_configs": [],
"exclude_iomgrs": [
"uv"
],
"flaky": false,
"gtest": false,
"language": "c",
"name": "ev_epollex_linux_test",
"platforms": [
"linux"
],
"uses_polling": true
},
{
"args": [],
"benchmark": false,

Loading…
Cancel
Save