Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(permissions): add PermissionsContainer struct for internal mutability #17134

Merged
merged 19 commits into from
Jan 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add PermissionsContainer helper type
  • Loading branch information
bartlomieju committed Dec 20, 2022
commit 67fa88af1487de6f34a46e6b54bb9b4953aea8b6
11 changes: 5 additions & 6 deletions cli/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ use crate::npm;

use deno_core::futures;
use deno_core::futures::FutureExt;
use deno_core::parking_lot::Mutex;
use deno_core::ModuleSpecifier;
use deno_graph::source::CacheInfo;
use deno_graph::source::LoadFuture;
use deno_graph::source::LoadResponse;
use deno_graph::source::Loader;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use std::sync::Arc;

mod check;
Expand Down Expand Up @@ -43,17 +42,17 @@ pub const CACHE_PERM: u32 = 0o644;
/// a concise interface to the DENO_DIR when building module graphs.
pub struct FetchCacher {
emit_cache: EmitCache,
dynamic_permissions: Arc<Mutex<Permissions>>,
dynamic_permissions: PermissionsContainer,
file_fetcher: Arc<FileFetcher>,
root_permissions: Arc<Mutex<Permissions>>,
root_permissions: PermissionsContainer,
}

impl FetchCacher {
pub fn new(
emit_cache: EmitCache,
file_fetcher: FileFetcher,
root_permissions: Arc<Mutex<Permissions>>,
dynamic_permissions: Arc<Mutex<Permissions>>,
root_permissions: PermissionsContainer,
dynamic_permissions: PermissionsContainer,
) -> Self {
let file_fetcher = Arc::new(file_fetcher);

Expand Down
9 changes: 5 additions & 4 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use deno_runtime::deno_fetch::reqwest::header::AUTHORIZATION;
use deno_runtime::deno_fetch::reqwest::header::IF_NONE_MATCH;
use deno_runtime::deno_fetch::reqwest::StatusCode;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use log::debug;
use std::borrow::Borrow;
use std::collections::HashMap;
Expand Down Expand Up @@ -403,7 +403,7 @@ impl FileFetcher {
fn fetch_remote(
&self,
specifier: &ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
redirect_limit: i64,
maybe_accept: Option<String>,
) -> Pin<Box<dyn Future<Output = Result<File, AnyError>> + Send>> {
Expand Down Expand Up @@ -542,7 +542,7 @@ impl FileFetcher {
pub async fn fetch(
&self,
specifier: &ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
) -> Result<File, AnyError> {
debug!("FileFetcher::fetch() - specifier: {}", specifier);
self.fetch_with_accept(specifier, permissions, None).await
Expand All @@ -551,7 +551,7 @@ impl FileFetcher {
pub async fn fetch_with_accept(
&self,
specifier: &ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
maybe_accept: Option<&str>,
) -> Result<File, AnyError> {
let scheme = get_validated_scheme(specifier)?;
Expand Down Expand Up @@ -728,6 +728,7 @@ mod tests {
use deno_runtime::deno_fetch::create_http_client;
use deno_runtime::deno_web::Blob;
use deno_runtime::deno_web::InMemoryBlobPart;
use deno_runtime::permissions::Permissions;
use std::fs::read;
use test_util::TempDir;

Expand Down
3 changes: 2 additions & 1 deletion cli/lsp/testing/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use deno_core::ModuleSpecifier;
use deno_runtime::ops::io::Stdio;
use deno_runtime::ops::io::StdioPipe;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::tokio_util::run_local;
use indexmap::IndexMap;
use std::collections::HashMap;
Expand Down Expand Up @@ -148,7 +149,7 @@ impl LspTestFilter {
#[allow(clippy::too_many_arguments)]
async fn test_specifier(
ps: proc_state::ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifier: ModuleSpecifier,
mode: test::TestMode,
sender: TestEventSender,
Expand Down
7 changes: 4 additions & 3 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use deno_core::ModuleType;
use deno_core::OpState;
use deno_core::SourceMapGetter;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use std::cell::RefCell;
use std::pin::Pin;
use std::rc::Rc;
Expand All @@ -40,7 +41,7 @@ pub struct CliModuleLoader {
/// The initial set of permissions used to resolve the static imports in the
/// worker. They are decoupled from the worker (dynamic) permissions since
/// read access errors must be raised based on the parent thread permissions.
pub root_permissions: Arc<Mutex<Permissions>>,
pub root_permissions: PermissionsContainer,
pub ps: ProcState,
}

Expand All @@ -55,7 +56,7 @@ impl CliModuleLoader {

pub fn new_for_worker(
ps: ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
) -> Rc<Self> {
Rc::new(CliModuleLoader {
lib: ps.options.ts_type_lib_worker(),
Expand Down Expand Up @@ -240,7 +241,7 @@ impl ModuleLoader for CliModuleLoader {
let ps = self.ps.clone();
let state = op_state.borrow();

let dynamic_permissions = state.borrow::<Arc<Mutex<Permissions>>>().clone();
let dynamic_permissions = state.borrow::<PermissionsContainer>().clone();
let root_permissions = if is_dynamic {
dynamic_permissions.clone()
} else {
Expand Down
10 changes: 5 additions & 5 deletions cli/ops/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use deno_core::ModuleSpecifier;
use deno_core::OpState;
use deno_runtime::permissions::create_child_permissions;
use deno_runtime::permissions::ChildPermissionsArg;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use serde::Deserialize;
use serde::Serialize;
use std::sync::atomic::AtomicUsize;
Expand Down Expand Up @@ -42,15 +42,15 @@ pub fn init(
}

#[derive(Clone)]
struct PermissionsHolder(Uuid, Arc<Mutex<Permissions>>);
struct PermissionsHolder(Uuid, PermissionsContainer);

#[op]
pub fn op_pledge_test_permissions(
state: &mut OpState,
args: ChildPermissionsArg,
) -> Result<Uuid, AnyError> {
let token = Uuid::new_v4();
let parent_permissions = state.borrow_mut::<Arc<Mutex<Permissions>>>();
let parent_permissions = state.borrow_mut::<PermissionsContainer>();
let worker_permissions = {
let mut parent_permissions = parent_permissions.lock();
let perms = create_child_permissions(&mut parent_permissions, args)?;
Expand All @@ -65,7 +65,7 @@ pub fn op_pledge_test_permissions(
state.put::<PermissionsHolder>(PermissionsHolder(token, parent_permissions));

// NOTE: This call overrides current permission set for the worker
state.put::<Arc<Mutex<Permissions>>>(worker_permissions);
state.put::<PermissionsContainer>(worker_permissions);

Ok(token)
}
Expand All @@ -81,7 +81,7 @@ pub fn op_restore_test_permissions(
}

let permissions = permissions_holder.1;
state.put::<Arc<Mutex<Permissions>>>(permissions);
state.put::<PermissionsContainer>(permissions);
Ok(())
} else {
Err(generic_error("no permissions to restore"))
Expand Down
10 changes: 5 additions & 5 deletions cli/ops/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use deno_core::ModuleSpecifier;
use deno_core::OpState;
use deno_runtime::permissions::create_child_permissions;
use deno_runtime::permissions::ChildPermissionsArg;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
Expand Down Expand Up @@ -52,15 +52,15 @@ pub fn init(
}

#[derive(Clone)]
struct PermissionsHolder(Uuid, Arc<Mutex<Permissions>>);
struct PermissionsHolder(Uuid, PermissionsContainer);

#[op]
pub fn op_pledge_test_permissions(
state: &mut OpState,
args: ChildPermissionsArg,
) -> Result<Uuid, AnyError> {
let token = Uuid::new_v4();
let parent_permissions = state.borrow_mut::<Arc<Mutex<Permissions>>>();
let parent_permissions = state.borrow_mut::<PermissionsContainer>();
let worker_permissions = {
let mut parent_permissions = parent_permissions.lock();
let perms = create_child_permissions(&mut parent_permissions, args)?;
Expand All @@ -74,7 +74,7 @@ pub fn op_pledge_test_permissions(
state.put::<PermissionsHolder>(PermissionsHolder(token, parent_permissions));

// NOTE: This call overrides current permission set for the worker
state.put::<Arc<Mutex<Permissions>>>(worker_permissions);
state.put::<PermissionsContainer>(worker_permissions);

Ok(token)
}
Expand All @@ -90,7 +90,7 @@ pub fn op_restore_test_permissions(
}

let permissions = permissions_holder.1;
state.put::<Arc<Mutex<Permissions>>>(permissions);
state.put::<PermissionsContainer>(permissions);
Ok(())
} else {
Err(generic_error("no permissions to restore"))
Expand Down
5 changes: 3 additions & 2 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use deno_runtime::deno_tls::rustls::RootCertStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use import_map::ImportMap;
use log::warn;
use std::collections::HashSet;
Expand Down Expand Up @@ -292,8 +293,8 @@ impl ProcState {
roots: Vec<ModuleSpecifier>,
is_dynamic: bool,
lib: TsTypeLib,
root_permissions: Arc<Mutex<Permissions>>,
dynamic_permissions: Arc<Mutex<Permissions>>,
root_permissions: PermissionsContainer,
dynamic_permissions: PermissionsContainer,
reload_on_watch: bool,
) -> Result<(), AnyError> {
log::debug!("Preparing module load.");
Expand Down
7 changes: 4 additions & 3 deletions cli/tools/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use deno_core::parking_lot::Mutex;
use deno_core::ModuleSpecifier;
use deno_graph::ModuleKind;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::tokio_util::run_local;
use indexmap::IndexMap;
use log::Level;
Expand Down Expand Up @@ -331,7 +332,7 @@ impl BenchReporter for ConsoleReporter {
/// Type check a collection of module and document specifiers.
async fn check_specifiers(
ps: &ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifiers: Vec<ModuleSpecifier>,
) -> Result<(), AnyError> {
let lib = ps.options.ts_type_lib_window();
Expand All @@ -351,7 +352,7 @@ async fn check_specifiers(
/// Run a single specifier as an executable bench module.
async fn bench_specifier(
ps: ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifier: ModuleSpecifier,
channel: UnboundedSender<BenchEvent>,
options: BenchSpecifierOptions,
Expand All @@ -372,7 +373,7 @@ async fn bench_specifier(
/// Test a collection of specifiers with test modes concurrently.
async fn bench_specifiers(
ps: ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifiers: Vec<ModuleSpecifier>,
options: BenchSpecifierOptions,
) -> Result<(), AnyError> {
Expand Down
7 changes: 4 additions & 3 deletions cli/tools/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use deno_runtime::fmt_errors::format_js_error;
use deno_runtime::ops::io::Stdio;
use deno_runtime::ops::io::StdioPipe;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::tokio_util::run_local;
use indexmap::IndexMap;
use log::Level;
Expand Down Expand Up @@ -711,7 +712,7 @@ pub fn format_test_error(js_error: &JsError) -> String {
/// both.
async fn test_specifier(
ps: ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifier: ModuleSpecifier,
mode: TestMode,
sender: TestEventSender,
Expand Down Expand Up @@ -921,7 +922,7 @@ async fn fetch_inline_files(
/// Type check a collection of module and document specifiers.
pub async fn check_specifiers(
ps: &ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifiers: Vec<(ModuleSpecifier, TestMode)>,
) -> Result<(), AnyError> {
let lib = ps.options.ts_type_lib_window();
Expand Down Expand Up @@ -988,7 +989,7 @@ pub async fn check_specifiers(
/// Test a collection of specifiers with test modes concurrently.
async fn test_specifiers(
ps: ProcState,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
specifiers_with_mode: Vec<(ModuleSpecifier, TestMode)>,
options: TestSpecifierOptions,
) -> Result<(), AnyError> {
Expand Down
11 changes: 6 additions & 5 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use deno_core::error::AnyError;
use deno_core::futures::task::LocalFutureObj;
use deno_core::futures::FutureExt;
use deno_core::located_script_name;
use deno_core::parking_lot::Mutex;
use deno_core::serde_json::json;
use deno_core::serde_v8;
use deno_core::v8;
Expand All @@ -17,7 +16,7 @@ use deno_runtime::colors;
use deno_runtime::fmt_errors::format_js_error;
use deno_runtime::ops::worker_host::CreateWebWorkerCb;
use deno_runtime::ops::worker_host::WorkerEventCb;
use deno_runtime::permissions::Permissions;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::web_worker::WebWorker;
use deno_runtime::web_worker::WebWorkerOptions;
use deno_runtime::worker::MainWorker;
Expand Down Expand Up @@ -411,7 +410,7 @@ impl CliMainWorker {
pub async fn create_main_worker(
ps: &ProcState,
main_module: ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
) -> Result<CliMainWorker, AnyError> {
create_main_worker_internal(
ps,
Expand All @@ -427,7 +426,7 @@ pub async fn create_main_worker(
pub async fn create_main_worker_for_test_or_bench(
ps: &ProcState,
main_module: ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
custom_extensions: Vec<Extension>,
stdio: deno_runtime::ops::io::Stdio,
) -> Result<CliMainWorker, AnyError> {
Expand All @@ -445,7 +444,7 @@ pub async fn create_main_worker_for_test_or_bench(
async fn create_main_worker_internal(
ps: &ProcState,
main_module: ModuleSpecifier,
permissions: Arc<Mutex<Permissions>>,
permissions: PermissionsContainer,
mut custom_extensions: Vec<Extension>,
stdio: deno_runtime::ops::io::Stdio,
bench_or_test: bool,
Expand Down Expand Up @@ -729,9 +728,11 @@ fn create_web_worker_callback(
#[cfg(test)]
mod tests {
use super::*;
use deno_core::parking_lot::Mutex;
use deno_core::{resolve_url_or_path, FsModuleLoader};
use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::Permissions;

fn create_test_worker() -> MainWorker {
let main_module = resolve_url_or_path("./hello.js").unwrap();
Expand Down
Loading