Skip to content

Commit e327e07

Browse files
committed
fix deadlock potential in schedule
There was a theoretical deadlock possible when an external thread would have filled the channel while a task on the event loop tries to schedule, potentially blocking the main thread. Additionally, when multiple requests schedule in parallel before MAIN_TID is initialized, they could deadlock (because they have to go via nginx_notify — although they come from main thread, we didn't capture MAIN_TID yet). Making the channel unbounded prevents that. After MAIN_TID is initialized, schedule will always .run(), not schedule.
1 parent 152c0ac commit e327e07

File tree

1 file changed

+11
-12
lines changed

1 file changed

+11
-12
lines changed

src/async_/spawn.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use std::sync::OnceLock;
55

66
use core::future::Future;
77

8+
use async_task::Runnable;
89
pub use async_task::Task;
9-
use async_task::{Runnable, ScheduleInfo, WithInfo};
10-
use crossbeam_channel::{bounded, Receiver, Sender};
10+
use crossbeam_channel::{unbounded, Receiver, Sender};
1111
use nginx_sys::{ngx_event_actions, ngx_event_t, ngx_thread_tid};
1212

1313
use crate::log::ngx_cycle_log;
@@ -64,19 +64,18 @@ struct Scheduler {
6464

6565
impl Scheduler {
6666
fn new() -> Self {
67-
let (tx, rx) = bounded(1);
67+
let (tx, rx) = unbounded();
6868
Scheduler { tx, rx }
6969
}
7070

71-
fn schedule(&self, runnable: Runnable, info: ScheduleInfo) {
72-
// are we on main thread and not woken_while_running? just .run()…
73-
if !info.woken_while_running && on_event_thread() {
71+
fn schedule(&self, runnable: Runnable) {
72+
// are we on main thread just .run()…
73+
if on_event_thread() {
7474
runnable.run();
7575
} else {
76-
// …otherwise we were called from some other thread, e.g. io handler,
77-
// or reentrantly (woken_while_running):
76+
// …otherwise we were called from some other thread, e.g. io handler:
7877
// ngx_notify to interrupt epoll and move it into the event loop
79-
self.tx.send(runnable).expect("send_blocking");
78+
self.tx.send(runnable).expect("send");
8079
notify();
8180
}
8281
}
@@ -88,9 +87,9 @@ fn scheduler() -> &'static Scheduler {
8887
SCHEDULER.get_or_init(Scheduler::new)
8988
}
9089

91-
fn schedule(runnable: Runnable, info: ScheduleInfo) {
90+
fn schedule(runnable: Runnable) {
9291
let scheduler = scheduler();
93-
scheduler.schedule(runnable, info);
92+
scheduler.schedule(runnable);
9493
}
9594

9695
/// Creates a new task running on the NGINX event loop.
@@ -101,7 +100,7 @@ where
101100
{
102101
ngx_log_debug!(ngx_cycle_log().as_ptr(), "async: spawning new task");
103102
// safe alternative: spawn_local, but this would check tid twice needlessly
104-
let (runnable, task) = unsafe { async_task::spawn_unchecked(future, WithInfo(schedule)) };
103+
let (runnable, task) = unsafe { async_task::spawn_unchecked(future, schedule) };
105104
runnable.schedule();
106105
task
107106
}

0 commit comments

Comments
 (0)