Merge pull request #13454 from yang-g/lockfree_event
Avoid calling dtor on grpc_fd before putting it in freelist
diff --git a/src/core/lib/iomgr/ev_epoll1_linux.cc b/src/core/lib/iomgr/ev_epoll1_linux.cc
index 60446f7..0dda1d9 100644
--- a/src/core/lib/iomgr/ev_epoll1_linux.cc
+++ b/src/core/lib/iomgr/ev_epoll1_linux.cc
@@ -263,11 +263,13 @@
if (new_fd == nullptr) {
new_fd = (grpc_fd*)gpr_malloc(sizeof(grpc_fd));
+ new_fd->read_closure.Init();
+ new_fd->write_closure.Init();
}
new_fd->fd = fd;
- new_fd->read_closure.Init();
- new_fd->write_closure.Init();
+ new_fd->read_closure->InitEvent();
+ new_fd->write_closure->InitEvent();
gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL);
new_fd->freelist_next = nullptr;
@@ -336,8 +338,8 @@
GRPC_CLOSURE_SCHED(exec_ctx, on_done, GRPC_ERROR_REF(error));
grpc_iomgr_unregister_object(&fd->iomgr_object);
- fd->read_closure.Destroy();
- fd->write_closure.Destroy();
+ fd->read_closure->DestroyEvent();
+ fd->write_closure->DestroyEvent();
gpr_mu_lock(&fd_freelist_mu);
fd->freelist_next = fd_freelist;
diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc
index 10b8440..62643df 100644
--- a/src/core/lib/iomgr/ev_epollex_linux.cc
+++ b/src/core/lib/iomgr/ev_epollex_linux.cc
@@ -286,8 +286,8 @@
fd->freelist_next = fd_freelist;
fd_freelist = fd;
- fd->read_closure.Destroy();
- fd->write_closure.Destroy();
+ fd->read_closure->DestroyEvent();
+ fd->write_closure->DestroyEvent();
gpr_mu_unlock(&fd_freelist_mu);
}
@@ -340,6 +340,8 @@
if (new_fd == nullptr) {
new_fd = (grpc_fd*)gpr_malloc(sizeof(grpc_fd));
+ new_fd->read_closure.Init();
+ new_fd->write_closure.Init();
}
gpr_mu_init(&new_fd->pollable_mu);
@@ -347,8 +349,8 @@
new_fd->pollable_obj = nullptr;
gpr_atm_rel_store(&new_fd->refst, (gpr_atm)1);
new_fd->fd = fd;
- new_fd->read_closure.Init();
- new_fd->write_closure.Init();
+ new_fd->read_closure->InitEvent();
+ new_fd->write_closure->InitEvent();
gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL);
new_fd->freelist_next = nullptr;
diff --git a/src/core/lib/iomgr/ev_epollsig_linux.cc b/src/core/lib/iomgr/ev_epollsig_linux.cc
index 689c0d4..12c8483 100644
--- a/src/core/lib/iomgr/ev_epollsig_linux.cc
+++ b/src/core/lib/iomgr/ev_epollsig_linux.cc
@@ -767,8 +767,8 @@
fd_freelist = fd;
grpc_iomgr_unregister_object(&fd->iomgr_object);
- fd->read_closure.Destroy();
- fd->write_closure.Destroy();
+ fd->read_closure->DestroyEvent();
+ fd->write_closure->DestroyEvent();
gpr_mu_unlock(&fd_freelist_mu);
} else {
@@ -819,6 +819,8 @@
if (new_fd == nullptr) {
new_fd = (grpc_fd*)gpr_malloc(sizeof(grpc_fd));
gpr_mu_init(&new_fd->po.mu);
+ new_fd->read_closure.Init();
+ new_fd->write_closure.Init();
}
/* Note: It is not really needed to get the new_fd->po.mu lock here. If this
@@ -833,8 +835,8 @@
gpr_atm_rel_store(&new_fd->refst, (gpr_atm)1);
new_fd->fd = fd;
new_fd->orphaned = false;
- new_fd->read_closure.Init();
- new_fd->write_closure.Init();
+ new_fd->read_closure->InitEvent();
+ new_fd->write_closure->InitEvent();
gpr_atm_no_barrier_store(&new_fd->read_notifier_pollset, (gpr_atm)NULL);
new_fd->freelist_next = nullptr;
diff --git a/src/core/lib/iomgr/lockfree_event.cc b/src/core/lib/iomgr/lockfree_event.cc
index d477f64..f0e798e 100644
--- a/src/core/lib/iomgr/lockfree_event.cc
+++ b/src/core/lib/iomgr/lockfree_event.cc
@@ -57,7 +57,9 @@
namespace grpc_core {
-LockfreeEvent::LockfreeEvent() {
+LockfreeEvent::LockfreeEvent() { InitEvent(); }
+
+void LockfreeEvent::InitEvent() {
/* Perform an atomic store to start the state machine.
Note carefully that LockfreeEvent *MAY* be used whilst in a destroyed
@@ -67,7 +69,7 @@
gpr_atm_no_barrier_store(&state_, kClosureNotReady);
}
-LockfreeEvent::~LockfreeEvent() {
+void LockfreeEvent::DestroyEvent() {
gpr_atm curr;
do {
curr = gpr_atm_no_barrier_load(&state_);
diff --git a/src/core/lib/iomgr/lockfree_event.h b/src/core/lib/iomgr/lockfree_event.h
index c667dcd..aec67a3 100644
--- a/src/core/lib/iomgr/lockfree_event.h
+++ b/src/core/lib/iomgr/lockfree_event.h
@@ -30,11 +30,16 @@
class LockfreeEvent {
public:
LockfreeEvent();
- ~LockfreeEvent();
LockfreeEvent(const LockfreeEvent&) = delete;
LockfreeEvent& operator=(const LockfreeEvent&) = delete;
+ // These methods are used to initialize and destroy the internal state. These
+ // cannot be done in constructor and destructor because SetReady may be called
+ // when the event is destroyed and put in a freelist.
+ void InitEvent();
+ void DestroyEvent();
+
bool IsShutdown() const {
return (gpr_atm_no_barrier_load(&state_) & kShutdownBit) != 0;
}