Skip to content

Commit 3780d0e

Browse files
authored
fix(hermes): thread_local sqlite resource management (#535)
* feat(hermes): update sqlite runtime extension to use thread-local storage, stop ignoring parallel_execution test * chore(hermes): refactor sqlite state management * feat(hermes): add sql validation for execute * chore(hermes): refactor code, so it uses RteInit traits for release, simplified host-side usage of resources, resolve comments * chore(hermes): pre-push fix * chore(hermes): remove allow(unused) for wal mode * chore(hermes): code refactoring, add comment for RteEvent * chore(hermes): remove comment after counter fix * fix(hermes): move closing of connections to drop * chore(hermes): rename core module to kernel so traitreg would not argue * fix(hermes): handle resource creation error during open * chore(hermes): reuse finalize in statement drop * fix(hermes): update wit for statement finalize, so it consumes the statement * fix(hermes): fix finalize usage in tests
1 parent 472db0e commit 3780d0e

File tree

21 files changed

+781
-109
lines changed

21 files changed

+781
-109
lines changed

hermes/apps/athena/modules/rbac-registration/src/lib.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ use crate::{
4545
utils::log::log_info,
4646
};
4747

48-
use hermes::cardano;
48+
use hermes::{cardano, sqlite::api::Statement};
4949

5050
struct RbacRegistrationComponent;
5151

@@ -115,8 +115,8 @@ impl exports::hermes::cardano::event_on_block::Guest for RbacRegistrationCompone
115115
}
116116
insert_rbac_registration(&rbac_stmt, rbac_data);
117117
}
118-
let _ = rbac_stmt.finalize();
119-
let _ = rbac_stake_stmt.finalize();
118+
let _ = Statement::finalize(rbac_stmt);
119+
let _ = Statement::finalize(rbac_stake_stmt);
120120

121121
close_db_connection(sqlite);
122122
}

hermes/bin/src/runtime_extensions/hermes/sqlite/connection/core.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ pub(crate) fn execute(
9393
db_ptr: *mut sqlite3,
9494
sql: &str,
9595
) -> Result<(), Errno> {
96+
if validate_sql(sql) {
97+
return Err(Errno::ForbiddenPragmaCommand);
98+
}
99+
96100
let sql_cstring = std::ffi::CString::new(sql).map_err(|_| Errno::ConvertingCString)?;
97101

98102
let rc = unsafe { sqlite3_exec(db_ptr, sql_cstring.as_ptr(), None, null_mut(), null_mut()) };
@@ -112,7 +116,7 @@ mod tests {
112116
runtime_extensions::{
113117
bindings::hermes::sqlite::api::Value,
114118
hermes::sqlite::{
115-
core::open,
119+
kernel::open,
116120
statement::core::{column, finalize, step},
117121
},
118122
},
@@ -146,6 +150,16 @@ mod tests {
146150

147151
assert!(matches!(stmt_ptr, Err(Errno::ForbiddenPragmaCommand)));
148152

153+
let sql = "pragma page_size;";
154+
let result = execute(db_ptr, sql);
155+
156+
assert!(matches!(result, Err(Errno::ForbiddenPragmaCommand)));
157+
158+
let sql = "PRAGMA page_size;";
159+
let result = execute(db_ptr, sql);
160+
161+
assert!(matches!(result, Err(Errno::ForbiddenPragmaCommand)));
162+
149163
close(db_ptr).unwrap();
150164
}
151165

hermes/bin/src/runtime_extensions/hermes/sqlite/connection/host.rs

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
//! `SQLite` connection object host implementation for WASM runtime.
44
5-
use super::{super::state::get_db_state, core};
5+
use super::core;
66
use crate::{
77
runtime_context::HermesRuntimeContext,
88
runtime_extensions::{
99
bindings::hermes::sqlite::api::{Errno, ErrorInfo, HostSqlite, Sqlite, Statement},
10-
hermes::sqlite::state::get_statement_state,
10+
hermes::sqlite::state::resource_manager,
1111
},
1212
};
1313

@@ -24,12 +24,10 @@ impl HostSqlite for HermesRuntimeContext {
2424
/// is automatically rolled back.
2525
fn close(
2626
&mut self,
27-
resource: wasmtime::component::Resource<Sqlite>,
27+
_resource: wasmtime::component::Resource<Sqlite>,
2828
) -> wasmtime::Result<Result<(), Errno>> {
29-
let app_state = get_db_state().get_app_state(self.app_name())?;
30-
let db_ptr = app_state.delete_resource(resource)?;
31-
32-
Ok(core::close(db_ptr as *mut _))
29+
// Connection close is handled automatically in `Drop` implementation
30+
Ok(Ok(()))
3331
}
3432

3533
/// Retrieves the numeric result code for the most recent failed `SQLite` operation on
@@ -42,10 +40,10 @@ impl HostSqlite for HermesRuntimeContext {
4240
&mut self,
4341
resource: wasmtime::component::Resource<Sqlite>,
4442
) -> wasmtime::Result<Option<ErrorInfo>> {
45-
let mut app_state = get_db_state().get_app_state(self.app_name())?;
46-
let db_ptr = app_state.get_object(&resource)?;
43+
let db_handle = resource.rep().try_into()?;
44+
let db_ptr = resource_manager::get_connection_pointer(self.app_name(), db_handle)?;
4745

48-
Ok(core::errcode(*db_ptr as *mut _))
46+
Ok(core::errcode(db_ptr as *mut _))
4947
}
5048

5149
/// Compiles SQL text into byte-code that will do the work of querying or updating the
@@ -66,18 +64,20 @@ impl HostSqlite for HermesRuntimeContext {
6664
resource: wasmtime::component::Resource<Sqlite>,
6765
sql: String,
6866
) -> wasmtime::Result<Result<wasmtime::component::Resource<Statement>, Errno>> {
69-
let mut db_app_state = get_db_state().get_app_state(self.app_name())?;
70-
let db_ptr = db_app_state.get_object(&resource)?;
67+
let db_handle = resource.rep().try_into()?;
68+
let db_ptr = resource_manager::get_connection_pointer(self.app_name(), db_handle)?;
7169

72-
let result = core::prepare(*db_ptr as *mut _, sql.as_str());
70+
let result = core::prepare(db_ptr as *mut _, sql.as_str());
7371

7472
match result {
7573
Ok(stmt_ptr) => {
7674
if stmt_ptr.is_null() {
7775
Ok(Err(Errno::ReturnedNullPointer))
7876
} else {
79-
let stm_app_state = get_statement_state().get_app_state(self.app_name())?;
80-
let stmt = stm_app_state.create_resource(stmt_ptr as _);
77+
let stmt = resource_manager::create_statement_resource(
78+
self.app_name(),
79+
stmt_ptr as _,
80+
)?;
8181

8282
Ok(Ok(stmt))
8383
}
@@ -97,21 +97,16 @@ impl HostSqlite for HermesRuntimeContext {
9797
resource: wasmtime::component::Resource<Sqlite>,
9898
sql: String,
9999
) -> wasmtime::Result<Result<(), Errno>> {
100-
let mut app_state = get_db_state().get_app_state(self.app_name())?;
101-
let db_ptr = app_state.get_object(&resource)?;
100+
let db_handle = resource.rep().try_into()?;
101+
let db_ptr = resource_manager::get_connection_pointer(self.app_name(), db_handle)?;
102102

103-
Ok(core::execute(*db_ptr as *mut _, sql.as_str()))
103+
Ok(core::execute(db_ptr as *mut _, sql.as_str()))
104104
}
105105

106106
fn drop(
107107
&mut self,
108-
rep: wasmtime::component::Resource<Sqlite>,
108+
_rep: wasmtime::component::Resource<Sqlite>,
109109
) -> wasmtime::Result<()> {
110-
let app_state = get_db_state().get_app_state(self.app_name())?;
111-
if let Ok(db_ptr) = app_state.delete_resource(rep) {
112-
let _ = core::close(db_ptr as *mut _);
113-
}
114-
115110
Ok(())
116111
}
117112
}

hermes/bin/src/runtime_extensions/hermes/sqlite/host.rs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
11
//! `SQLite` host implementation for WASM runtime.
22
3-
use super::{core, state::get_db_state};
43
use crate::{
54
runtime_context::HermesRuntimeContext,
6-
runtime_extensions::bindings::hermes::sqlite::api::{Errno, Host, Sqlite},
5+
runtime_extensions::{
6+
bindings::hermes::sqlite::api::{Errno, Host, Sqlite},
7+
hermes::sqlite::{
8+
connection::core::close,
9+
kernel,
10+
state::{connection::DbHandle, resource_manager},
11+
},
12+
},
713
};
814

915
impl Host for HermesRuntimeContext {
@@ -24,12 +30,33 @@ impl Host for HermesRuntimeContext {
2430
readonly: bool,
2531
memory: bool,
2632
) -> wasmtime::Result<Result<wasmtime::component::Resource<Sqlite>, Errno>> {
27-
match core::open(readonly, memory, self.app_name().clone()) {
28-
Ok(db_ptr) => {
29-
let app_state = get_db_state().get_app_state(self.app_name())?;
30-
let db_id = app_state.create_resource(db_ptr as _);
33+
let db_handle = DbHandle::from_readonly_and_memory(readonly, memory);
34+
35+
if let Some(resource) =
36+
resource_manager::try_get_connection_resource(self.app_name(), db_handle)
37+
{
38+
return Ok(Ok(resource));
39+
}
3140

32-
Ok(Ok(db_id))
41+
match kernel::open(readonly, memory, self.app_name().clone()) {
42+
Ok(db_ptr) => {
43+
match resource_manager::create_connection_resource(
44+
self.app_name(),
45+
db_handle,
46+
db_ptr as _,
47+
) {
48+
Ok(resource) => Ok(Ok(resource)),
49+
Err(err) => {
50+
if let Err(errno) = close(db_ptr) {
51+
anyhow::bail!(
52+
"failed to create connection resource: {err}, also failed to close the connection with errno: {errno}..."
53+
)
54+
}
55+
anyhow::bail!(
56+
"failed to create connection resource: {err}, closing the connection..."
57+
)
58+
},
59+
}
3360
},
3461
Err(err) => Ok(Err(err)),
3562
}

hermes/bin/src/runtime_extensions/hermes/sqlite/core.rs renamed to hermes/bin/src/runtime_extensions/hermes/sqlite/kernel.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ extern "C" fn busy_handler(
8080
/// # Returns
8181
/// * `Ok(())` on success.
8282
/// * `Err(Errno)` if any of the configuration steps fail.
83-
#[allow(unused)]
8483
fn enable_wal_mode(db_ptr: *mut sqlite3) -> Result<(), Errno> {
8584
let pragmas = "PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;";
8685
let c_pragmas = std::ffi::CString::new(pragmas).map_err(|_| Errno::ConvertingCString)?;
@@ -163,8 +162,7 @@ pub(super) fn open(
163162
}
164163

165164
if !memory && !readonly {
166-
// TODO(anyone): fix issue with locks during hermes app execution https://github.com/input-output-hk/hermes/issues/521
167-
// enable_wal_mode(db_ptr)?;
165+
enable_wal_mode(db_ptr)?;
168166
}
169167

170168
// config database size limitation
Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,67 @@
11
//! `SQLite` runtime extension implementation.
22
3+
use tracing::debug;
4+
5+
use crate::{
6+
app::ApplicationName,
7+
runtime_context::HermesRuntimeContext,
8+
runtime_extensions::{
9+
hermes::sqlite::state::resource_manager::init_app_state,
10+
init::{
11+
errors::RteInitResult, trait_app::RteInitApp, trait_event::RteInitEvent,
12+
trait_module::RteInitModule, trait_runtime::RteInitRuntime,
13+
},
14+
},
15+
wasm::module::ModuleId,
16+
};
17+
318
mod connection;
4-
mod core;
519
mod host;
20+
mod kernel;
621
mod state;
722
mod statement;
823

9-
/// Advise Runtime Extensions of a new context
10-
pub(crate) fn new_context(ctx: &crate::runtime_context::HermesRuntimeContext) {
11-
state::get_db_state().add_app(ctx.app_name().clone());
12-
state::get_statement_state().add_app(ctx.app_name().clone());
24+
/// Runtime Extension for `SQLite`
25+
#[derive(Default)]
26+
struct RteSqlite;
27+
28+
#[traitreg::register(default)]
29+
impl RteInitRuntime for RteSqlite {}
30+
31+
#[traitreg::register(default)]
32+
impl RteInitApp for RteSqlite {}
33+
34+
#[traitreg::register(default)]
35+
impl RteInitModule for RteSqlite {
36+
fn init(
37+
self: Box<Self>,
38+
name: &ApplicationName,
39+
module_id: &ModuleId,
40+
) -> RteInitResult {
41+
debug!(name=%name, module_id=%module_id,"Hermes Runtime Extensions Initialized: Module");
42+
init_app_state(name);
43+
44+
Ok(())
45+
}
46+
}
47+
48+
#[traitreg::register(default)]
49+
impl RteInitEvent for RteSqlite {
50+
fn init(
51+
self: Box<Self>,
52+
ctx: &HermesRuntimeContext,
53+
) -> RteInitResult {
54+
debug!(
55+
name=%ctx.app_name(),
56+
module=%ctx.module_id(),
57+
event=%ctx.event_name(),
58+
exc_count=%ctx.exc_counter(),
59+
"Hermes Runtime Extensions Initialized: Event");
60+
61+
init_app_state(ctx.app_name());
62+
Ok(())
63+
}
1364
}
65+
66+
/// Advise Runtime Extensions of a new context
67+
pub(crate) fn new_context(_ctx: &crate::runtime_context::HermesRuntimeContext) {}

hermes/bin/src/runtime_extensions/hermes/sqlite/state.rs

Lines changed: 0 additions & 33 deletions
This file was deleted.

0 commit comments

Comments
 (0)