mirror of
https://github.com/denoland/deno.git
synced 2025-01-11 16:42:21 -05:00
fix(runtime): event loop panics in classic workers (#11756)
Classic worker scripts are now executed in the context of a Tokio runtime. This does mean we can not spawn more tokio runtimes in "op_worker_sync_fetch". We instead spawn a new thread there, that can create a new Tokio runtime that we can use to block the worker thread.
This commit is contained in:
parent
480cfda8d5
commit
a66218d457
6 changed files with 144 additions and 131 deletions
|
@ -1109,6 +1109,13 @@ itest!(import_file_with_colon {
|
||||||
output: "import_file_with_colon.ts.out",
|
output: "import_file_with_colon.ts.out",
|
||||||
http_server: true,
|
http_server: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
itest!(classic_workers_event_loop {
|
||||||
|
args:
|
||||||
|
"run --enable-testing-features-do-not-use classic_workers_event_loop.js",
|
||||||
|
output: "classic_workers_event_loop.js.out",
|
||||||
|
});
|
||||||
|
|
||||||
// FIXME(bartlomieju): disabled, because this test is very flaky on CI
|
// FIXME(bartlomieju): disabled, because this test is very flaky on CI
|
||||||
// itest!(local_sources_not_cached_in_memory {
|
// itest!(local_sources_not_cached_in_memory {
|
||||||
// args: "run --allow-read --allow-write no_mem_cache.js",
|
// args: "run --allow-read --allow-write no_mem_cache.js",
|
||||||
|
|
4
cli/tests/testdata/classic_workers_event_loop.js
vendored
Normal file
4
cli/tests/testdata/classic_workers_event_loop.js
vendored
Normal file
|
@ -0,0 +1,4 @@
|
||||||
|
new Worker(
|
||||||
|
"data:application/javascript,setTimeout(() => {console.log('done'); self.close()}, 1000)",
|
||||||
|
{ type: "classic" },
|
||||||
|
);
|
1
cli/tests/testdata/classic_workers_event_loop.js.out
vendored
Normal file
1
cli/tests/testdata/classic_workers_event_loop.js.out
vendored
Normal file
|
@ -0,0 +1 @@
|
||||||
|
done
|
|
@ -38,29 +38,31 @@ pub fn op_worker_sync_fetch(
|
||||||
let handle = state.borrow::<WebWorkerInternalHandle>().clone();
|
let handle = state.borrow::<WebWorkerInternalHandle>().clone();
|
||||||
assert_eq!(handle.worker_type, WebWorkerType::Classic);
|
assert_eq!(handle.worker_type, WebWorkerType::Classic);
|
||||||
|
|
||||||
// TODO(andreubotella) Make the runtime into a resource and add a new op to
|
// TODO(andreubotella) It's not good to throw an exception related to blob
|
||||||
// block on each request, so a script can run while the next loads.
|
// URLs when none of the script URLs use the blob scheme.
|
||||||
|
// Also, in which contexts are blob URLs not supported?
|
||||||
|
let blob_store = state
|
||||||
|
.try_borrow::<BlobStore>()
|
||||||
|
.ok_or_else(|| type_error("Blob URLs are not supported in this context."))?
|
||||||
|
.clone();
|
||||||
|
|
||||||
|
// TODO(andreubotella): make the below thread into a resource that can be
|
||||||
|
// re-used. This would allow parallel fecthing of multiple scripts.
|
||||||
|
|
||||||
|
let thread = std::thread::spawn(move || {
|
||||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||||
.enable_io()
|
.enable_io()
|
||||||
.enable_time()
|
.enable_time()
|
||||||
.build()
|
.build()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
// TODO(andreubotella) It's not good to throw an exception related to blob
|
|
||||||
// URLs when none of the script URLs use the blob scheme.
|
|
||||||
// Also, in which contexts are blob URLs not supported?
|
|
||||||
let blob_store = state.try_borrow::<BlobStore>().ok_or_else(|| {
|
|
||||||
type_error("Blob URLs are not supported in this context.")
|
|
||||||
})?;
|
|
||||||
|
|
||||||
let handles: Vec<_> = scripts
|
let handles: Vec<_> = scripts
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|script| -> JoinHandle<Result<SyncFetchScript, AnyError>> {
|
.map(|script| -> JoinHandle<Result<SyncFetchScript, AnyError>> {
|
||||||
let blob_store = blob_store.clone();
|
let blob_store = blob_store.clone();
|
||||||
runtime.spawn(async move {
|
runtime.spawn(async move {
|
||||||
let script_url =
|
let script_url = Url::parse(&script)
|
||||||
Url::parse(&script).map_err(|_| type_error("Invalid script URL"))?;
|
.map_err(|_| type_error("Invalid script URL"))?;
|
||||||
|
|
||||||
let (body, mime_type, res_url) = match script_url.scheme() {
|
let (body, mime_type, res_url) = match script_url.scheme() {
|
||||||
"http" | "https" => {
|
"http" | "https" => {
|
||||||
|
@ -98,9 +100,10 @@ pub fn op_worker_sync_fetch(
|
||||||
(Bytes::from(body), Some(mime_type), script)
|
(Bytes::from(body), Some(mime_type), script)
|
||||||
}
|
}
|
||||||
"blob" => {
|
"blob" => {
|
||||||
let blob = blob_store
|
let blob =
|
||||||
.get_object_url(script_url)?
|
blob_store.get_object_url(script_url)?.ok_or_else(|| {
|
||||||
.ok_or_else(|| type_error("Blob for the given URL not found."))?;
|
type_error("Blob for the given URL not found.")
|
||||||
|
})?;
|
||||||
|
|
||||||
let mime_type = mime_type_essence(&blob.media_type);
|
let mime_type = mime_type_essence(&blob.media_type);
|
||||||
|
|
||||||
|
@ -152,4 +155,7 @@ pub fn op_worker_sync_fetch(
|
||||||
ret.push(script);
|
ret.push(script);
|
||||||
}
|
}
|
||||||
Ok(ret)
|
Ok(ret)
|
||||||
|
});
|
||||||
|
|
||||||
|
thread.join().unwrap()
|
||||||
}
|
}
|
||||||
|
|
|
@ -11,7 +11,6 @@ use deno_core::error::AnyError;
|
||||||
use deno_core::error::JsError;
|
use deno_core::error::JsError;
|
||||||
use deno_core::futures::channel::mpsc;
|
use deno_core::futures::channel::mpsc;
|
||||||
use deno_core::futures::future::poll_fn;
|
use deno_core::futures::future::poll_fn;
|
||||||
use deno_core::futures::future::FutureExt;
|
|
||||||
use deno_core::futures::stream::StreamExt;
|
use deno_core::futures::stream::StreamExt;
|
||||||
use deno_core::located_script_name;
|
use deno_core::located_script_name;
|
||||||
use deno_core::serde::Deserialize;
|
use deno_core::serde::Deserialize;
|
||||||
|
@ -568,15 +567,14 @@ pub fn run_web_worker(
|
||||||
// TODO(bartlomieju): run following block using "select!"
|
// TODO(bartlomieju): run following block using "select!"
|
||||||
// with terminate
|
// with terminate
|
||||||
|
|
||||||
|
let fut = async move {
|
||||||
// Execute provided source code immediately
|
// Execute provided source code immediately
|
||||||
let result = if let Some(source_code) = maybe_source_code {
|
let result = if let Some(source_code) = maybe_source_code {
|
||||||
worker.execute_script(&located_script_name!(), &source_code)
|
worker.execute_script(&located_script_name!(), &source_code)
|
||||||
} else {
|
} else {
|
||||||
// TODO(bartlomieju): add "type": "classic", ie. ability to load
|
// TODO(bartlomieju): add "type": "classic", ie. ability to load
|
||||||
// script instead of module
|
// script instead of module
|
||||||
let load_future = worker.execute_module(&specifier).boxed_local();
|
worker.execute_module(&specifier).await
|
||||||
|
|
||||||
rt.block_on(load_future)
|
|
||||||
};
|
};
|
||||||
|
|
||||||
let internal_handle = worker.internal_handle.clone();
|
let internal_handle = worker.internal_handle.clone();
|
||||||
|
@ -597,7 +595,10 @@ pub fn run_web_worker(
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
let result = rt.block_on(worker.run_event_loop(true));
|
let result = worker.run_event_loop(true).await;
|
||||||
debug!("Worker thread shuts down {}", &name);
|
debug!("Worker thread shuts down {}", &name);
|
||||||
result
|
result
|
||||||
|
};
|
||||||
|
|
||||||
|
rt.block_on(fut)
|
||||||
}
|
}
|
||||||
|
|
|
@ -16744,13 +16744,7 @@
|
||||||
"Cross-origin throw",
|
"Cross-origin throw",
|
||||||
"Redirect-to-cross-origin syntax error",
|
"Redirect-to-cross-origin syntax error",
|
||||||
"Redirect-to-Cross-origin throw"
|
"Redirect-to-Cross-origin throw"
|
||||||
],
|
]
|
||||||
"report-error-cross-origin.sub.any.worker.html": false,
|
|
||||||
"report-error-redirect-to-cross-origin.sub.any.worker.html": false,
|
|
||||||
"report-error-same-origin.sub.any.worker.html": false,
|
|
||||||
"report-error-setTimeout-cross-origin.sub.any.worker.html": false,
|
|
||||||
"report-error-setTimeout-redirect-to-cross-origin.sub.any.worker.html": false,
|
|
||||||
"report-error-setTimeout-same-origin.sub.any.worker.html": false
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
|
Loading…
Reference in a new issue