Skip to content

Commit 239b070

Browse files
committed
Fix asset_debug_server hang. There should be at most one ThreadExecut… (bevyengine#7825)
…or's ticker for one thread. # Objective - Fix debug_asset_server hang. ## Solution - Reuse the thread_local executor for MainThreadExecutor resource, so there will be only one ThreadExecutor for main thread. - If ThreadTickers from same executor, they are conflict with each other. Then only tick one.
1 parent e54103f commit 239b070

File tree

4 files changed

+45
-14
lines changed

4 files changed

+45
-14
lines changed

crates/bevy_ecs/src/schedule/executor/multi_threaded.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -620,11 +620,17 @@ fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &World
620620
}
621621

622622
/// New-typed [`ThreadExecutor`] [`Resource`] that is used to run systems on the main thread
623-
#[derive(Resource, Default, Clone)]
623+
#[derive(Resource, Clone)]
624624
pub struct MainThreadExecutor(pub Arc<ThreadExecutor<'static>>);
625625

626+
impl Default for MainThreadExecutor {
627+
fn default() -> Self {
628+
Self::new()
629+
}
630+
}
631+
626632
impl MainThreadExecutor {
627633
pub fn new() -> Self {
628-
MainThreadExecutor(Arc::new(ThreadExecutor::new()))
634+
MainThreadExecutor(TaskPool::get_thread_executor())
629635
}
630636
}

crates/bevy_tasks/src/single_threaded_task_pool.rs

+5
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ impl TaskPoolBuilder {
5656
pub struct TaskPool {}
5757

5858
impl TaskPool {
59+
/// Just create a new `ThreadExecutor` for wasm
60+
pub fn get_thread_executor() -> Arc<ThreadExecutor<'static>> {
61+
Arc::new(ThreadExecutor::new())
62+
}
63+
5964
/// Create a `TaskPool` with the default configuration.
6065
pub fn new() -> Self {
6166
TaskPoolBuilder::new().build()

crates/bevy_tasks/src/task_pool.rs

+23-8
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,12 @@ pub struct TaskPool {
112112
impl TaskPool {
113113
thread_local! {
114114
static LOCAL_EXECUTOR: async_executor::LocalExecutor<'static> = async_executor::LocalExecutor::new();
115-
static THREAD_EXECUTOR: ThreadExecutor<'static> = ThreadExecutor::new();
115+
static THREAD_EXECUTOR: Arc<ThreadExecutor<'static>> = Arc::new(ThreadExecutor::new());
116+
}
117+
118+
/// Each thread should only create one `ThreadExecutor`, otherwise, there are good chances they will deadlock
119+
pub fn get_thread_executor() -> Arc<ThreadExecutor<'static>> {
120+
Self::THREAD_EXECUTOR.with(|executor| executor.clone())
116121
}
117122

118123
/// Create a `TaskPool` with the default configuration.
@@ -376,24 +381,34 @@ impl TaskPool {
376381
let tick_task_pool_executor = tick_task_pool_executor || self.threads.is_empty();
377382

378383
// we get this from a thread local so we should always be on the scope executors thread.
384+
// note: it is possible `scope_executor` and `external_executor` is the same executor,
385+
// in that case, we should only tick one of them, otherwise, it may cause deadlock.
379386
let scope_ticker = scope_executor.ticker().unwrap();
380-
if let Some(external_ticker) = external_executor.ticker() {
381-
if tick_task_pool_executor {
387+
let external_ticker = if !external_executor.is_same(scope_executor) {
388+
external_executor.ticker()
389+
} else {
390+
None
391+
};
392+
393+
match (external_ticker, tick_task_pool_executor) {
394+
(Some(external_ticker), true) => {
382395
Self::execute_global_external_scope(
383396
executor,
384397
external_ticker,
385398
scope_ticker,
386399
get_results,
387400
)
388401
.await
389-
} else {
402+
}
403+
(Some(external_ticker), false) => {
390404
Self::execute_external_scope(external_ticker, scope_ticker, get_results)
391405
.await
392406
}
393-
} else if tick_task_pool_executor {
394-
Self::execute_global_scope(executor, scope_ticker, get_results).await
395-
} else {
396-
Self::execute_scope(scope_ticker, get_results).await
407+
// either external_executor is none or it is same as scope_executor
408+
(None, true) => {
409+
Self::execute_global_scope(executor, scope_ticker, get_results).await
410+
}
411+
(None, false) => Self::execute_scope(scope_ticker, get_results).await,
397412
}
398413
})
399414
}

crates/bevy_tasks/src/thread_executor.rs

+9-4
Original file line numberDiff line numberDiff line change
@@ -77,33 +77,38 @@ impl<'task> ThreadExecutor<'task> {
7777
pub fn ticker<'ticker>(&'ticker self) -> Option<ThreadExecutorTicker<'task, 'ticker>> {
7878
if thread::current().id() == self.thread_id {
7979
return Some(ThreadExecutorTicker {
80-
executor: &self.executor,
80+
executor: self,
8181
_marker: PhantomData::default(),
8282
});
8383
}
8484
None
8585
}
86+
87+
/// Returns true if `self` and `other`'s executor is same
88+
pub fn is_same(&self, other: &Self) -> bool {
89+
std::ptr::eq(self, other)
90+
}
8691
}
8792

8893
/// Used to tick the [`ThreadExecutor`]. The executor does not
8994
/// make progress unless it is manually ticked on the thread it was
9095
/// created on.
9196
#[derive(Debug)]
9297
pub struct ThreadExecutorTicker<'task, 'ticker> {
93-
executor: &'ticker Executor<'task>,
98+
executor: &'ticker ThreadExecutor<'task>,
9499
// make type not send or sync
95100
_marker: PhantomData<*const ()>,
96101
}
97102
impl<'task, 'ticker> ThreadExecutorTicker<'task, 'ticker> {
98103
/// Tick the thread executor.
99104
pub async fn tick(&self) {
100-
self.executor.tick().await;
105+
self.executor.executor.tick().await;
101106
}
102107

103108
/// Synchronously try to tick a task on the executor.
104109
/// Returns false if if does not find a task to tick.
105110
pub fn try_tick(&self) -> bool {
106-
self.executor.try_tick()
111+
self.executor.executor.try_tick()
107112
}
108113
}
109114

0 commit comments

Comments
 (0)