From 045041566f60f9d933a052dee22034f36550ab4f Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 30 Oct 2026 15:13:21 +0800 Subject: [PATCH] fix(storage): use temporary disk backing without a profile --- Cargo.lock | 1 + moli-core/Cargo.toml | 1 + moli-core/src/runtime/storage_partition.rs | 99 +++++++++++++++++-- .../storage_partition/tests/shutdown_tests.rs | 33 ++++++- .../src/protocol_server/mod.rs | 4 +- moli-protocol/src/conn.rs | 29 +++++- .../src/conn/state/browser_context.rs | 68 ++++++++++--- .../src/conn/tests/resource_runtime.rs | 28 ++++-- .../src/domains/network/tests/cache.rs | 4 +- .../src/domains/target/browser_context.rs | 9 +- moli/src/cli.rs | 1 + 11 files changed, 235 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 926db0eb52..58d8257829 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2781,6 +2781,7 @@ dependencies = [ "serde_json", "static_assertions", "strip-ansi-escapes", + "tempfile", "thiserror", "tokio", "tracing", diff --git a/moli-core/Cargo.toml b/moli-core/Cargo.toml index 0ffca939c1..fcc7e3dfdb 100644 --- a/moli-core/Cargo.toml +++ b/moli-core/Cargo.toml @@ -31,6 +31,7 @@ moli-fetch = { path = "../moli-fetch" } parking_lot = "0.12" serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.145" +tempfile = "3.27.0" tokio = { version = "1.51.0", features = ["macros", "net", "rt-multi-thread", "signal", "sync", "time"] } thiserror = "2.0.17" tracing = "0.1.41" diff --git a/moli-core/src/runtime/storage_partition.rs b/moli-core/src/runtime/storage_partition.rs index 81145c2d01..ab1b6606ac 100644 --- a/moli-core/src/runtime/storage_partition.rs +++ b/moli-core/src/runtime/storage_partition.rs @@ -31,7 +31,7 @@ use crate::{ #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum StoragePartitionPersistence { - Memory, + Ephemeral, ProfileBacked, } @@ -47,6 +47,9 @@ pub struct StoragePartitionState { service_worker_resource_store: SharedServiceWorkerResourceStore, http_cache_root: Option, profile: Option>, + // Drop after storage handles: temporary disk backing has partition scope, + // while an explicit profile keeps its files after the partition closes. + temporary_directory: Option, } #[derive(Clone)] @@ -90,6 +93,13 @@ impl fmt::Debug for StoragePartitionState { ) .field("http_cache_root", &self.http_cache_root) .field("profile_backed", &self.profile.is_some()) + .field( + "temporary_root", + &self + .temporary_directory + .as_ref() + .map(tempfile::TempDir::path), + ) .finish() } } @@ -104,6 +114,16 @@ impl StoragePartitionState { } fn from_profile(profile: Option>) -> Result { + let temporary_directory = if profile.is_none() { + Some( + tempfile::Builder::new() + .prefix("moli-storage-") + .tempdir() + .context("failed to create temporary browser storage directory")?, + ) + } else { + None + }; let cookie_store = new_shared_browser_cookie_store(); let session_storage_store = new_shared_web_storage_store(); let ( @@ -193,18 +213,26 @@ impl StoragePartitionState { Some(partition.http_cache_root().to_path_buf()), ) } else { - let indexed_db_manager = - moli_renderer_v8::new_sqlite_indexed_db_manager(None, Default::default()) - .map_err(|error| anyhow!(error)) - .context("failed to initialize in-memory IndexedDB")?; - let storage_service = StorageService::in_memory(); + let root = temporary_directory + .as_ref() + .expect("an ephemeral partition owns its temporary directory") + .path(); + let indexed_db_manager = moli_renderer_v8::new_sqlite_indexed_db_manager( + Some(root.join("idb-lite")), + Default::default(), + ) + .map_err(|error| anyhow!(error)) + .context("failed to initialize temporary IndexedDB")?; + let storage_service = StorageService::on_disk(root.join("opfs")) + .map_err(|error| anyhow!(error)) + .context("failed to initialize temporary OPFS")?; let storage_bucket_store = new_shared_storage_bucket_store_with_storage_service_and_indexed_db_manager( storage_service.clone(), &indexed_db_manager, ); ( - StoragePartitionPersistence::Memory, + StoragePartitionPersistence::Ephemeral, "default", new_shared_web_storage_store(), indexed_db_manager, @@ -226,6 +254,7 @@ impl StoragePartitionState { service_worker_resource_store, http_cache_root, profile, + temporary_directory, }) } @@ -503,22 +532,72 @@ mod tests { } #[test] - fn memory_partition_owns_ephemeral_stores() -> Result<()> { + fn ephemeral_partition_owns_temporary_stores() -> Result<()> { let partition = StoragePartitionState::open(None)?; + let root = partition + .temporary_directory + .as_ref() + .unwrap() + .path() + .to_path_buf(); - assert_eq!(partition.persistence(), StoragePartitionPersistence::Memory); + assert_eq!( + partition.persistence(), + StoragePartitionPersistence::Ephemeral + ); assert_eq!(partition.id(), "default"); assert!(partition.http_cache_root().is_none()); assert!(partition.profile_cookie_cache_paths().is_empty()); let service = partition.storage_service.clone(); let facade_service = partition.storage_bucket_store().lock().storage_service(); assert!(Arc::ptr_eq(&service, &facade_service)); + assert!(root.join("idb-lite/format.json").is_file()); + assert!(!root.join("opfs").exists(), "OPFS remains lazy"); + + drop(partition); + assert!( + !root.exists(), + "normal partition teardown removes temporary data" + ); Ok(()) } #[test] - fn memory_partition_flush_is_noop() -> Result<()> { + fn ephemeral_partition_opfs_is_disk_backed_and_partition_isolated() -> Result<()> { + let first = StoragePartitionState::open(None)?; + let second = StoragePartitionState::open(None)?; + let root = first.temporary_directory.as_ref().unwrap().path(); + assert_ne!(root, second.temporary_directory.as_ref().unwrap().path()); + let locator = StorageBucketLocator::default_bucket("https://temporary-opfs.test"); + let key = moli_storage_service::StorageService::opfs_bucket_key(&locator)?; + let directory = first.storage_service.ensure_opfs_root(&locator)?; + let file = first.storage_service.with_opfs(|opfs| { + let file = opfs.get_file(&key, &directory, "entry.txt", true)?; + opfs.write_file(&key, &file, b"temporary bytes", None)?; + Ok::<_, moli_storage_service::OpfsError>(file) + })?; + // A separate backend must see the bytes through the host filesystem, + // without sharing the first service's in-memory namespace. + let reopened = moli_storage_service::StorageService::on_disk(root.join("opfs"))?; + assert_eq!( + reopened + .with_opfs(|opfs| opfs.read_file(&key, &file))? + .bytes, + b"temporary bytes", + ); + let other_directory = second.storage_service.ensure_opfs_root(&locator)?; + assert!( + second + .storage_service + .with_opfs(|opfs| { opfs.get_file(&key, &other_directory, "entry.txt", false) }) + .is_err() + ); + Ok(()) + } + + #[test] + fn ephemeral_partition_cookie_flush_is_noop() -> Result<()> { let partition = StoragePartitionState::open(None)?; partition.import_cookies(vec![stored_cookie("sid", "memory")])?; diff --git a/moli-core/src/runtime/storage_partition/tests/shutdown_tests.rs b/moli-core/src/runtime/storage_partition/tests/shutdown_tests.rs index f578b94abd..3c69f7f27c 100644 --- a/moli-core/src/runtime/storage_partition/tests/shutdown_tests.rs +++ b/moli-core/src/runtime/storage_partition/tests/shutdown_tests.rs @@ -218,8 +218,14 @@ fn partition_shutdown_finishes_accepted_bucket_cleanup_before_closing_sql() { } #[test] -fn default_memory_partition_uses_sql_and_retained_clients_cannot_keep_its_data_alive() { +fn temporary_partition_stores_sql_on_disk_and_retires_retained_clients_before_cleanup() { let partition = StoragePartitionState::open(None).unwrap(); + let root = partition + .temporary_directory + .as_ref() + .unwrap() + .path() + .to_path_buf(); let service = partition.indexed_db_manager.lock().sqlite_service(); let client = database(&service, b"default-memory"); upgrade(&client, 1, "kept"); @@ -230,9 +236,34 @@ fn default_memory_partition_uses_sql_and_retained_clients_cannot_keep_its_data_a quota: None, }, ); + let mut directories = vec![root.join("idb-lite")]; + let mut databases = Vec::new(); + while let Some(directory) = directories.pop() { + for entry in fs::read_dir(directory).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + directories.push(path); + } else if path + .extension() + .is_some_and(|extension| extension == "sqlite") + { + databases.push(path); + } + } + } + assert_eq!(databases.len(), 1); + assert!( + fs::read(&databases[0]) + .unwrap() + .starts_with(b"SQLite format 3\0") + ); drop(partition); assert!(service.is_closed()); assert!(client.is_retired()); + assert!( + !root.exists(), + "retained SQL clients must not pin temporary files" + ); let partition = StoragePartitionState::open(None).unwrap(); let service = partition.indexed_db_manager.lock().sqlite_service(); assert_eq!( diff --git a/moli-protocol-server/src/protocol_server/mod.rs b/moli-protocol-server/src/protocol_server/mod.rs index 1a0d20d53a..3baa1fd54e 100644 --- a/moli-protocol-server/src/protocol_server/mod.rs +++ b/moli-protocol-server/src/protocol_server/mod.rs @@ -82,11 +82,11 @@ impl ProtocolServer { initial_cookies: Vec, ) -> Self { let storage_partition = Arc::new( - StoragePartitionState::open(None).expect("in-memory storage partition should open"), + StoragePartitionState::open(None).expect("temporary storage partition should open"), ); storage_partition .import_cookies(initial_cookies) - .expect("in-memory cookie import should succeed"); + .expect("initial cookie import should succeed"); Self::new_with_storage_partition(config, storage_partition) } diff --git a/moli-protocol/src/conn.rs b/moli-protocol/src/conn.rs index 0e74ea9ce0..043bd29113 100644 --- a/moli-protocol/src/conn.rs +++ b/moli-protocol/src/conn.rs @@ -1002,10 +1002,17 @@ pub struct CdpInitialStoragePartition { } impl CdpInitialStoragePartition { + /// Explicit in-memory backing for embedders and backend-focused tests. pub fn memory() -> Self { Self::new(BrowserContextStoragePartitionHandles::memory()) } + pub fn temporary() -> anyhow::Result { + Ok(Self::new( + BrowserContextStoragePartitionHandles::temporary()? + )) + } + pub fn with_cookies(cookies: Vec) -> Self { Self::new(BrowserContextStoragePartitionHandles::with_initial_cookies( cookies, @@ -1383,6 +1390,10 @@ impl Drop for CdpConnection { contexts.push(context); } contexts.append(&mut self.inactive_browser_contexts); + let storage_partitions = contexts + .iter() + .map(BrowserContext::retain_storage_for_teardown) + .collect::>(); let roots = contexts .iter_mut() .filter_map(BrowserContext::take_renderer_runtime_owner_for_teardown) @@ -1407,12 +1418,16 @@ impl Drop for CdpConnection { root.shutdown_network_and_join(); } drop(roots); + drop(storage_partitions); } } impl CdpConnection { pub fn new() -> Self { - Self::new_with_initial_storage_partition(CdpInitialStoragePartition::memory()) + Self::new_with_initial_storage_partition( + CdpInitialStoragePartition::temporary() + .expect("temporary browser storage should initialize"), + ) } pub(crate) fn layout_policy(&self) -> LayoutPolicy { @@ -1493,7 +1508,8 @@ impl CdpConnection { pub fn new_with_fetch_config(fetch_config: FetchConfig) -> Self { Self::new_with_initial_storage_partition_and_fetch_config( - CdpInitialStoragePartition::memory(), + CdpInitialStoragePartition::temporary() + .expect("temporary browser storage should initialize"), fetch_config, ) } @@ -3195,7 +3211,10 @@ impl CdpConnection { browser_context.global_geolocation_override = self.global_geolocation_override.clone(); } - pub(crate) fn new_ephemeral_browser_context(&self, id: String) -> BrowserContext { + pub(crate) fn new_ephemeral_browser_context( + &self, + id: String, + ) -> Result { let mut browser_context = BrowserContext::new_ephemeral_with_http_cache( id, self.fetch_config().http_cache_dir().map(PathBuf::from), @@ -3203,9 +3222,9 @@ impl CdpConnection { self.standalone_navigation_engine .runtime_config() .debugger_enabled(), - ); + )?; self.apply_global_browser_context_state(&mut browser_context); - browser_context + Ok(browser_context) } pub fn snapshot_cookies(&mut self) -> Vec { diff --git a/moli-protocol/src/conn/state/browser_context.rs b/moli-protocol/src/conn/state/browser_context.rs index a3cce14b8c..44b1d3e790 100644 --- a/moli-protocol/src/conn/state/browser_context.rs +++ b/moli-protocol/src/conn/state/browser_context.rs @@ -40,7 +40,6 @@ use super::{ pub struct BrowserContext { pub id: String, pub(crate) closing: bool, - storage_partition: BrowserContextStoragePartition, pub(crate) page_targets: PageTargetRegistry, /// Browser focus is independent of which context is selected for routing. pub(crate) window_is_focused: bool, @@ -86,9 +85,10 @@ pub struct BrowserContext { pub(crate) http_cache_max_bytes: Option, pub(crate) page_navigation_runtime_config: Option, renderer_output_transport_sender: Option, - // Keep the sole strong per-context network owner last. Protocol teardown - // must drop pages and NavigationEngines before this field is released. + // Release pages and NavigationEngines before joining the network owner, + // then close storage before deleting its temporary disk backing. pub(crate) renderer_runtime_owner: Option, + storage_partition: BrowserContextStoragePartition, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -115,6 +115,7 @@ pub(crate) struct BrowserContextStoragePartitionHandles { web_storage_store: SharedWebStorageStore, indexed_db_manager: SharedIndexedDbManager, storage_bucket_store: SharedStorageBucketStore, + _owner: Option>, } #[derive(Clone)] @@ -226,18 +227,12 @@ impl BrowserContextStoragePartitionHandles { web_storage_store, indexed_db_manager, storage_bucket_store, + _owner: None, } } pub(crate) fn memory() -> Self { - Self::with_initial_cookies(Vec::new()) - } - - pub(crate) fn with_initial_cookies( - initial_cookies: impl IntoIterator, - ) -> Self { let cookie_store = new_shared_browser_cookie_store(); - seed_initial_cookies(&cookie_store, initial_cookies); let indexed_db_manager = moli_core::storage::new_indexed_db_manager(None) .expect("in-memory IndexedDB manager should initialize"); let storage_bucket_store = @@ -250,6 +245,21 @@ impl BrowserContextStoragePartitionHandles { ) } + pub(crate) fn temporary() -> anyhow::Result { + let owner = std::sync::Arc::new(StoragePartitionState::open(None)?); + let mut handles = Self::from_storage_partition(Vec::new(), &owner); + handles._owner = Some(owner); + Ok(handles) + } + + pub(crate) fn with_initial_cookies( + initial_cookies: impl IntoIterator, + ) -> Self { + let handles = Self::temporary().expect("temporary browser storage should initialize"); + seed_initial_cookies(&handles.cookie_store, initial_cookies); + handles + } + fn from_initial_storage_partition( cookie_store: SharedBrowserCookieStore, local_storage_store: SharedWebStorageStore, @@ -457,14 +467,16 @@ impl BrowserContext { http_cache_root: Option, http_cache_max_bytes: Option, debugger_enabled: bool, - ) -> Self { - Self::new_ephemeral_with_storage_partition_handles( + ) -> Result { + let partition = BrowserContextStoragePartitionHandles::temporary() + .map_err(|error| format!("{error:#}"))?; + Ok(Self::new_ephemeral_with_storage_partition_handles( id, - BrowserContextStoragePartitionHandles::memory(), + partition, http_cache_root, http_cache_max_bytes, debugger_enabled, - ) + )) } pub(crate) fn new_with_storage_partition_and_http_cache( @@ -819,6 +831,10 @@ impl BrowserContext { self.renderer_runtime_owner.take() } + pub(crate) fn retain_storage_for_teardown(&self) -> BrowserContextStoragePartitionHandles { + self.storage_partition.handles.clone() + } + pub(crate) fn routes_renderer_browser_context_runtime( &self, runtime_id: moli_core::RendererBrowserContextRuntimeId, @@ -1803,3 +1819,27 @@ pub(crate) fn seed_initial_cookies( let _ = store.upsert_with_request_url_report(cookie, None, CookieSource::Cdp); } } + +#[cfg(test)] +mod temporary_storage_tests { + use super::*; + + #[test] + fn temporary_storage_handles_keep_the_partition_until_the_last_clone_drops() { + let handles = BrowserContextStoragePartitionHandles::temporary().unwrap(); + let owner = std::sync::Arc::downgrade(handles._owner.as_ref().unwrap()); + let sql = handles.indexed_db_manager.lock().sqlite_service(); + let retained = handles.clone(); + + drop(handles); + assert!(owner.upgrade().is_some()); + assert!(!sql.is_closed()); + + drop(retained); + assert!(owner.upgrade().is_none()); + assert!( + sql.is_closed(), + "retained SQL clients must not own the partition" + ); + } +} diff --git a/moli-protocol/src/conn/tests/resource_runtime.rs b/moli-protocol/src/conn/tests/resource_runtime.rs index 227c6fd54d..55620e7751 100644 --- a/moli-protocol/src/conn/tests/resource_runtime.rs +++ b/moli-protocol/src/conn/tests/resource_runtime.rs @@ -284,7 +284,9 @@ fn default_browser_contexts_reuse_partition_with_distinct_target_session_storage fn connection_ephemeral_browser_context_uses_isolated_storage_partition() { let conn = CdpConnection::new_with_initial_cookies(vec![stored_cookie("sid", "seeded")]); - let bc = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let bc = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); assert!(!bc.is_profile_backed_storage_partition()); assert_eq!(bc.storage_partition_kind_label(), "ephemeral"); @@ -300,7 +302,9 @@ fn connection_default_and_ephemeral_context_creation_use_named_partition_paths() let conn = CdpConnection::new_with_initial_storage_partition(initial_storage_partition); let profile_backed = conn.new_browser_context("BID-profile".to_owned()); - let ephemeral = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let ephemeral = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); let expected_web_storage_store = storage_partition .shared_storage_handles() .web_storage_store(); @@ -324,7 +328,9 @@ fn connection_default_and_ephemeral_context_creation_use_named_partition_paths() fn browser_context_memory_diagnostics_include_storage_partition_identity() { let conn = CdpConnection::new(); let profile_backed = conn.new_browser_context("BID-profile".to_owned()); - let ephemeral = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let ephemeral = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); assert_eq!( profile_backed.storage_partition_kind_label(), @@ -568,7 +574,9 @@ fn connection_profile_backed_cookie_snapshot_ignores_ephemeral_contexts() { let mut conn = CdpConnection::new(); let profile_backed = conn.new_browser_context("BID-profile".to_owned()); profile_backed.upsert_cookie_for_test(stored_cookie("profile", "1")); - let ephemeral = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let ephemeral = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); ephemeral.upsert_cookie_for_test(stored_cookie("ephemeral", "1")); conn.install_browser_context_fixture_for_test(ephemeral); conn.push_inactive_browser_context_fixture_for_test(profile_backed); @@ -584,7 +592,9 @@ fn connection_profile_backed_cookie_snapshot_ignores_ephemeral_contexts() { #[test] fn connection_profile_backed_cookie_snapshot_is_none_without_profile_backed_context() { let mut conn = CdpConnection::new_with_initial_cookies(vec![stored_cookie("sid", "seeded")]); - let ephemeral = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let ephemeral = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); assert!(ephemeral.snapshot_cookies().is_empty()); conn.install_browser_context_fixture_for_test(ephemeral); @@ -896,8 +906,12 @@ async fn browser_context_storage_does_not_cross_context_switches() { #[tokio::test] async fn browser_context_storage_buckets_reuse_within_context_and_isolate_between_contexts() { let mut conn = CdpConnection::new(); - let context_a = conn.new_ephemeral_browser_context("BID-1".into()); - let context_b = conn.new_ephemeral_browser_context("BID-2".into()); + let context_a = conn + .new_ephemeral_browser_context("BID-1".into()) + .expect("temporary context"); + let context_b = conn + .new_ephemeral_browser_context("BID-2".into()) + .expect("temporary context"); conn.install_browser_context_fixture_for_test(context_a); conn.push_inactive_browser_context_fixture_for_test(context_b); let first_url = Url::parse("https://context-storage-buckets.example/app/one").unwrap(); diff --git a/moli-protocol/src/domains/network/tests/cache.rs b/moli-protocol/src/domains/network/tests/cache.rs index 4d133ca743..d09a4e5d6b 100644 --- a/moli-protocol/src/domains/network/tests/cache.rs +++ b/moli-protocol/src/domains/network/tests/cache.rs @@ -606,7 +606,9 @@ fn new_ephemeral_browser_context_inherits_effective_http_cache_owner() { fetch_config.set_http_cache_max_bytes(Some(77)); let conn = crate::conn::CdpConnection::new_with_fetch_config(fetch_config); - let browser_context = conn.new_ephemeral_browser_context("BID-ephemeral".to_owned()); + let browser_context = conn + .new_ephemeral_browser_context("BID-ephemeral".to_owned()) + .expect("temporary context"); assert_eq!( browser_context.http_cache_root.as_deref(), diff --git a/moli-protocol/src/domains/target/browser_context.rs b/moli-protocol/src/domains/target/browser_context.rs index d8cff12e9f..1d45207414 100644 --- a/moli-protocol/src/domains/target/browser_context.rs +++ b/moli-protocol/src/domains/target/browser_context.rs @@ -292,7 +292,9 @@ pub(super) fn execute_devtools_create_browser_context_command( )); } - let mut browser_context = conn.new_ephemeral_browser_context(id.clone()); + let mut browser_context = conn + .new_ephemeral_browser_context(id.clone()) + .map_err(|error| DevToolsError::new(DevToolsErrorKind::Internal, error))?; browser_context.default_tls_verify_host_override = command.accept_insecure_certs.map(|accept| !accept); browser_context.default_http_proxy_override = command.proxy_server; @@ -505,7 +507,10 @@ pub(super) fn create_browser_context(conn: &mut CdpConnection, cmd: &Cmd<'_>) -> return CommandOutputPlan::error_without_session(-32602, message); } let id = conn.gen_bc_id(); - let mut browser_context = conn.new_ephemeral_browser_context(id.clone()); + let mut browser_context = match conn.new_ephemeral_browser_context(id.clone()) { + Ok(context) => context, + Err(error) => return CommandOutputPlan::error_without_session(-32000, &error), + }; browser_context.default_http_proxy_override = params.proxy_server; browser_context.default_http_no_proxy_override = normalize_proxy_bypass_list_for_loader(params.proxy_bypass_list.as_deref()); diff --git a/moli/src/cli.rs b/moli/src/cli.rs index f32545a593..a9d4b55db2 100644 --- a/moli/src/cli.rs +++ b/moli/src/cli.rs @@ -558,6 +558,7 @@ pub struct CommonArgs { #[arg(long)] pub http_cache_dir: Option, + /// Persist browser storage in this directory. Without it, IndexedDB and OPFS use temporary files. #[arg(short = 'P', long)] pub profile_dir: Option,