From 28f64171cb4292cc1e8cf59525b0b9990eff160f Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Sat, 20 Jan 2024 21:58:37 +0530 Subject: fix(node): use `cppgc` for managing X509Certificate (#21999) Introduces the first cppgc backed Resource into Deno. This fixes the memory leak when using `X509Certificate` **Comparison**: ```js import { X509Certificate } from 'node:crypto'; const r = Deno.readFileSync('cli/tests/node_compat/test/fixtures/keys/agent1-cert.pem'); setInterval(() => { for (let i = 0; i < 10000; i++) { const cert = new X509Certificate(r); } }, 1000); ``` Memory usage after 5 secs `main`: 1692MB `cppgc`: peaks at 400MB --- ext/node/ops/crypto/x509.rs | 125 +++++++++++++++----------------------------- 1 file changed, 41 insertions(+), 84 deletions(-) (limited to 'ext/node/ops') diff --git a/ext/node/ops/crypto/x509.rs b/ext/node/ops/crypto/x509.rs index b5779ffa9..440e7c291 100644 --- a/ext/node/ops/crypto/x509.rs +++ b/ext/node/ops/crypto/x509.rs @@ -3,10 +3,7 @@ use deno_core::error::bad_resource_id; use deno_core::error::AnyError; use deno_core::op2; -use deno_core::OpState; -use deno_core::Resource; - -use std::borrow::Cow; +use deno_core::v8; use x509_parser::der_parser::asn1_rs::Any; use x509_parser::der_parser::asn1_rs::Tag; @@ -48,17 +45,11 @@ impl std::ops::Deref for Certificate { } } -impl Resource for Certificate { - fn name(&self) -> Cow { - "x509Certificate".into() - } -} - -#[op2(fast)] -pub fn op_node_x509_parse( - state: &mut OpState, +#[op2] +pub fn op_node_x509_parse<'s>( + scope: &'s mut v8::HandleScope, #[buffer] buf: &[u8], -) -> Result { +) -> Result, AnyError> { let pem = match pem::parse_x509_pem(buf) { Ok((_, pem)) => Some(pem), Err(_) => None, @@ -76,32 +67,25 @@ pub fn op_node_x509_parse( cert: unsafe { std::mem::transmute(cert) }, pem, }; - let rid = state.resource_table.add(cert); - Ok(rid) + + let obj = deno_core::cppgc::make_cppgc_object(scope, cert); + Ok(obj) } #[op2(fast)] -pub fn op_node_x509_ca( - state: &mut OpState, - rid: u32, -) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; +pub fn op_node_x509_ca(rid: v8::Local) -> Result { + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.is_ca()) } #[op2(fast)] pub fn op_node_x509_check_email( - state: &mut OpState, - rid: u32, + rid: v8::Local, #[string] email: &str, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; let subject = cert.subject(); if subject @@ -137,65 +121,50 @@ pub fn op_node_x509_check_email( #[op2] #[string] pub fn op_node_x509_fingerprint( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result, AnyError> { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.fingerprint::()) } #[op2] #[string] pub fn op_node_x509_fingerprint256( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result, AnyError> { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.fingerprint::()) } #[op2] #[string] pub fn op_node_x509_fingerprint512( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result, AnyError> { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.fingerprint::()) } #[op2] #[string] pub fn op_node_x509_get_issuer( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(x509name_to_string(cert.issuer(), oid_registry())?) } #[op2] #[string] pub fn op_node_x509_get_subject( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(x509name_to_string(cert.subject(), oid_registry())?) } @@ -262,39 +231,30 @@ fn x509name_to_string( #[op2] #[string] pub fn op_node_x509_get_valid_from( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.validity().not_before.to_string()) } #[op2] #[string] pub fn op_node_x509_get_valid_to( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; Ok(cert.validity().not_after.to_string()) } #[op2] #[string] pub fn op_node_x509_get_serial_number( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; let mut s = cert.serial.to_str_radix(16); s.make_ascii_uppercase(); Ok(s) @@ -302,13 +262,10 @@ pub fn op_node_x509_get_serial_number( #[op2(fast)] pub fn op_node_x509_key_usage( - state: &mut OpState, - rid: u32, + rid: v8::Local, ) -> Result { - let cert = state - .resource_table - .get::(rid) - .map_err(|_| bad_resource_id())?; + let cert = deno_core::cppgc::unwrap_cppgc_object::(rid) + .ok_or_else(bad_resource_id)?; let key_usage = cert .extensions() -- cgit v1.2.3