This is an automated email from the ASF dual-hosted git repository.
cliffjansen pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
The following commit(s) were added to refs/heads/main by this push:
new c6db2b0 PROTON-2483: epoll proactor TSAN race - ensure task locks not
held when calling pni_timer_set()
c6db2b0 is described below
commit c6db2b089c6c40c46b5e37a701164bbce3f5ebf9
Author: Cliff Jansen <[email protected]>
AuthorDate: Wed Jan 12 13:00:14 2022 -0800
PROTON-2483: epoll proactor TSAN race - ensure task locks not held when
calling pni_timer_set()
---
c/src/proactor/epoll-internal.h | 3 ++-
c/src/proactor/epoll.c | 36 ++++++++++++++++++++++++++++--------
c/src/proactor/epoll_timer.c | 14 +++++++++-----
3 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/c/src/proactor/epoll-internal.h b/c/src/proactor/epoll-internal.h
index 984724f..8db12a1 100644
--- a/c/src/proactor/epoll-internal.h
+++ b/c/src/proactor/epoll-internal.h
@@ -157,6 +157,7 @@ struct pn_proactor_t {
bool need_timeout;
bool timeout_set; /* timeout has been set by user and not yet cancelled or
generated event */
bool timeout_processed; /* timeout event dispatched in the most recent
event batch */
+ pmutex timeout_mutex;
int task_count;
// ready list subsystem
@@ -391,7 +392,7 @@ void pni_raw_connection_done(praw_connection_t *rc);
pni_timer_t *pni_timer(pni_timer_manager_t *tm, pconnection_t *c);
void pni_timer_free(pni_timer_t *timer);
-void pni_timer_set(pni_timer_t *timer, uint64_t deadline);
+bool pni_timer_set(pni_timer_t *timer, uint64_t deadline);
bool pni_timer_manager_init(pni_timer_manager_t *tm);
void pni_timer_manager_finalize(pni_timer_manager_t *tm);
pn_event_batch_t *pni_timer_manager_process(pni_timer_manager_t *tm, bool
timeout, bool sched_ready);
diff --git a/c/src/proactor/epoll.c b/c/src/proactor/epoll.c
index 3784d05..d2f394a 100644
--- a/c/src/proactor/epoll.c
+++ b/c/src/proactor/epoll.c
@@ -835,8 +835,8 @@ static void pconnection_final_free(pconnection_t *pc) {
pmutex_finalize(&pc->rearm_mutex);
pn_condition_free(pc->disconnect_condition);
pn_connection_driver_destroy(&pc->driver);
- task_finalize(&pc->task);
pni_timer_free(pc->timer);
+ task_finalize(&pc->task);
free(pc);
}
@@ -1453,7 +1453,8 @@ static void pconnection_tick(pconnection_t *pc) {
lock(&pc->task.mutex);
pc->expected_timeout = next;
unlock(&pc->task.mutex);
- pni_timer_set(pc->timer, next);
+ if (pni_timer_set(pc->timer, next))
+ notify_poller(pc->task.proactor);
}
}
}
@@ -1957,6 +1958,7 @@ pn_proactor_t *pn_proactor() {
pmutex_init(&p->eventfd_mutex);
pmutex_init(&p->sched_mutex);
pmutex_init(&p->tslot_mutex);
+ pmutex_init(&p->timeout_mutex);
if ((p->epollfd = epoll_create(1)) >= 0) {
if ((p->eventfd = eventfd(0, EFD_NONBLOCK)) >= 0) {
@@ -1979,6 +1981,7 @@ pn_proactor_t *pn_proactor() {
if (p->eventfd >= 0) close(p->eventfd);
if (p->interruptfd >= 0) close(p->interruptfd);
pni_timer_manager_finalize(&p->timer_manager);
+ pmutex_finalize(&p->timeout_mutex);
pmutex_finalize(&p->tslot_mutex);
pmutex_finalize(&p->sched_mutex);
pmutex_finalize(&p->eventfd_mutex);
@@ -2014,6 +2017,7 @@ void pn_proactor_free(pn_proactor_t *p) {
pni_timer_manager_finalize(&p->timer_manager);
pn_collector_free(p->collector);
+ pmutex_finalize(&p->timeout_mutex);
pmutex_finalize(&p->tslot_mutex);
pmutex_finalize(&p->sched_mutex);
pmutex_finalize(&p->eventfd_mutex);
@@ -2768,26 +2772,42 @@ void pn_proactor_interrupt(pn_proactor_t *p) {
void pn_proactor_set_timeout(pn_proactor_t *p, pn_millis_t t) {
bool notify = false;
+ lock(&p->timeout_mutex);
+
lock(&p->task.mutex);
p->timeout_set = true;
if (t == 0) {
- pni_timer_set(p->timer, 0);
p->need_timeout = true;
- notify = schedule(&p->task);
- } else {
- pni_timer_set(p->timer, t + pn_proactor_now_64());
+ if (schedule(&p->task))
+ notify = true;
}
unlock(&p->task.mutex);
+
+ if (t == 0) {
+ if (pni_timer_set(p->timer, 0))
+ notify = true;
+ } else {
+ if (pni_timer_set(p->timer, t + pn_proactor_now_64()))
+ notify = true;
+ }
+ unlock(&p->timeout_mutex);
if (notify) notify_poller(p);
}
void pn_proactor_cancel_timeout(pn_proactor_t *p) {
+ bool notify = false;
+ lock(&p->timeout_mutex);
+
lock(&p->task.mutex);
p->timeout_set = false;
p->need_timeout = false;
- pni_timer_set(p->timer, 0);
- bool notify = schedule_if_inactive(p);
+ if (schedule_if_inactive(p))
+ notify = true;
unlock(&p->task.mutex);
+
+ if (pni_timer_set(p->timer, 0))
+ notify = true;
+ unlock(&p->timeout_mutex);
if (notify) notify_poller(p);
}
diff --git a/c/src/proactor/epoll_timer.c b/c/src/proactor/epoll_timer.c
index c154881..4a68ce1 100644
--- a/c/src/proactor/epoll_timer.c
+++ b/c/src/proactor/epoll_timer.c
@@ -153,7 +153,9 @@ pni_timer_t *pni_timer(pni_timer_manager_t *tm,
pconnection_t *c) {
void pni_timer_free(pni_timer_t *timer) {
timer_deadline_t *td = timer->timer_deadline;
bool can_free_td = false;
- if (td) pni_timer_set(timer, 0);
+ bool notify = false;
+ if (td)
+ notify = pni_timer_set(timer, 0);
pni_timer_manager_t *tm = timer->manager;
lock(&tm->task.mutex);
lock(&tm->deletion_mutex);
@@ -165,6 +167,8 @@ void pni_timer_free(pni_timer_t *timer) {
}
unlock(&tm->deletion_mutex);
unlock(&tm->task.mutex);
+ if (notify)
+ notify_poller(tm->task.proactor);
if (can_free_td) {
timer_deadline_t_free(td);
}
@@ -253,14 +257,15 @@ static bool adjust_deadline(pni_timer_manager_t *tm) {
// Call without task lock or timer_manager lock.
// Calls for connection timers are generated in the proactor and serialized
per connection.
// Calls for the proactor timer can come from arbitrary user threads.
-void pni_timer_set(pni_timer_t *timer, uint64_t deadline) {
+// Caller must call notify_poller() if true returned.
+bool pni_timer_set(pni_timer_t *timer, uint64_t deadline) {
pni_timer_manager_t *tm = timer->manager;
bool notify = false;
lock(&tm->task.mutex);
if (deadline == timer->deadline) {
unlock(&tm->task.mutex);
- return; // No change.
+ return false; // No change.
}
if (timer == tm->proactor_timer) {
@@ -293,8 +298,7 @@ void pni_timer_set(pni_timer_t *timer, uint64_t deadline) {
notify = adjust_deadline(tm);
unlock(&tm->task.mutex);
- if (notify)
- notify_poller(tm->task.proactor);
+ return notify;
}
pn_event_batch_t *pni_timer_manager_process(pni_timer_manager_t *tm, bool
timeout, bool sched_ready) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
