Skip to content

Commit

Permalink
Revert "fix: CFunctionInfo and CTypeInfo leaks (denoland#24634)" (den…
Browse files Browse the repository at this point in the history
…oland#24665)

This reverts commit 6c5905d.

Reverting because of errors like this in CI:
```
#
# Fatal error in , line 0
# Check failed: FastApiCallNode::ArityForArgc(c_arg_count, slow_arg_count) == value_input_count.
#
#
#
#FailureMessage Object: 0x7fd5b3ffd9d0
==== C stack trace ===============================

    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1111883) [0x55b8d2107883]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1110b6b) [0x55b8d2106b6b]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x110c6f9) [0x55b8d21026f9]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x20413cd) [0x55b8d30373cd]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x2034222) [0x55b8d302a222]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x202ff44) [0x55b8d3025f44]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1e0b253) [0x55b8d2e01253]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1dfcd96) [0x55b8d2df2d96]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1df92aa) [0x55b8d2def2aa]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1df8b71) [0x55b8d2deeb71]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x11ce49b) [0x55b8d21c449b]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x121fb23) [0x55b8d2215b23]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1221359) [0x55b8d2217359]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x1112f3b) [0x55b8d2108f3b]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x111792b) [0x55b8d210d92b]
    /home/runner/work/deno/deno/target/release/deps/url_ops-e4068c650cedb9fa(+0x110df0f) [0x55b8d2103f0f]
    /lib/x86_64-linux-gnu/libc.so.6(+0x94ac3) [0x7fd5cb294ac3]
    /lib/x86_64-linux-gnu/libc.so.6(+0x126850) [0x7fd5cb326850]
```
  • Loading branch information
bartlomieju authored Jul 22, 2024
1 parent 994b632 commit 95847f4
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 17 deletions.
16 changes: 8 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ repository = "https://github.com/denoland/deno"

[workspace.dependencies]
deno_ast = { version = "=0.40.0", features = ["transpiling"] }
deno_core = { version = "0.296.0" }
deno_core = { version = "0.294.0" }

deno_bench_util = { version = "0.155.0", path = "./bench_util" }
deno_lockfile = "0.20.0"
Expand Down
10 changes: 8 additions & 2 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use deno_core::ModuleType;
use deno_core::RequestedModuleType;
use deno_core::ResolutionKind;
use deno_core::SourceCodeCacheInfo;
use deno_core::SourceMapGetter;
use deno_graph::source::ResolutionMode;
use deno_graph::source::Resolver;
use deno_graph::GraphKind;
Expand Down Expand Up @@ -292,7 +293,8 @@ impl CliModuleLoaderFactory {
shared: self.shared.clone(),
})));
ModuleLoaderAndSourceMapGetter {
module_loader: loader,
module_loader: loader.clone(),
source_map_getter: Some(loader),
}
}
}
Expand Down Expand Up @@ -826,7 +828,11 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
}
std::future::ready(()).boxed_local()
}
}

impl<TGraphContainer: ModuleGraphContainer> SourceMapGetter
for CliModuleLoader<TGraphContainer>
{
fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>> {
let specifier = resolve_url(file_name).ok()?;
match specifier.scheme() {
Expand All @@ -839,7 +845,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
source_map_from_code(source.code.as_bytes())
}

fn get_source_mapped_source_line(
fn get_source_line(
&self,
file_name: &str,
line_number: usize,
Expand Down
2 changes: 2 additions & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ impl ModuleLoaderFactory for StandaloneModuleLoaderFactory {
root_permissions,
dynamic_permissions,
}),
source_map_getter: None,
}
}

Expand All @@ -399,6 +400,7 @@ impl ModuleLoaderFactory for StandaloneModuleLoaderFactory {
root_permissions,
dynamic_permissions,
}),
source_map_getter: None,
}
}
}
Expand Down
21 changes: 15 additions & 6 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use deno_core::ModuleId;
use deno_core::ModuleLoader;
use deno_core::PollEventLoopOptions;
use deno_core::SharedArrayBufferStore;
use deno_core::SourceMapGetter;
use deno_runtime::code_cache;
use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_runtime::deno_fs;
Expand Down Expand Up @@ -54,6 +55,7 @@ use crate::version;

pub struct ModuleLoaderAndSourceMapGetter {
pub module_loader: Rc<dyn ModuleLoader>,
pub source_map_getter: Option<Rc<dyn SourceMapGetter>>,
}

pub trait ModuleLoaderFactory: Send + Sync {
Expand Down Expand Up @@ -514,7 +516,10 @@ impl CliMainWorkerFactory {
(main_module, false)
};

let ModuleLoaderAndSourceMapGetter { module_loader } = shared
let ModuleLoaderAndSourceMapGetter {
module_loader,
source_map_getter,
} = shared
.module_loader_factory
.create_for_main(PermissionsContainer::allow_all(), permissions.clone());
let maybe_inspector_server = shared.maybe_inspector_server.clone();
Expand Down Expand Up @@ -591,6 +596,7 @@ impl CliMainWorkerFactory {
.clone(),
root_cert_store_provider: Some(shared.root_cert_store_provider.clone()),
seed: shared.options.seed,
source_map_getter,
format_js_error_fn: Some(Arc::new(format_js_error)),
create_web_worker_cb,
maybe_inspector_server,
Expand Down Expand Up @@ -724,11 +730,13 @@ fn create_web_worker_callback(
Arc::new(move |args| {
let maybe_inspector_server = shared.maybe_inspector_server.clone();

let ModuleLoaderAndSourceMapGetter { module_loader } =
shared.module_loader_factory.create_for_worker(
args.parent_permissions.clone(),
args.permissions.clone(),
);
let ModuleLoaderAndSourceMapGetter {
module_loader,
source_map_getter,
} = shared.module_loader_factory.create_for_worker(
args.parent_permissions.clone(),
args.permissions.clone(),
);
let create_web_worker_cb =
create_web_worker_callback(mode, shared.clone(), stdio.clone());

Expand Down Expand Up @@ -794,6 +802,7 @@ fn create_web_worker_callback(
seed: shared.options.seed,
create_web_worker_cb,
format_js_error_fn: Some(Arc::new(format_js_error)),
source_map_getter,
module_loader,
fs: shared.fs.clone(),
node_resolver: Some(shared.node_resolver.clone()),
Expand Down
3 changes: 3 additions & 0 deletions runtime/web_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use deno_core::ModuleSpecifier;
use deno_core::PollEventLoopOptions;
use deno_core::RuntimeOptions;
use deno_core::SharedArrayBufferStore;
use deno_core::SourceMapGetter;
use deno_cron::local::LocalCronHandler;
use deno_fs::FileSystem;
use deno_http::DefaultHttpPropertyExtractor;
Expand Down Expand Up @@ -368,6 +369,7 @@ pub struct WebWorkerOptions {
pub npm_resolver: Option<Arc<dyn deno_node::NpmResolver>>,
pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
pub format_js_error_fn: Option<Arc<FormatJsErrorFn>>,
pub source_map_getter: Option<Rc<dyn SourceMapGetter>>,
pub worker_type: WebWorkerType,
pub maybe_inspector_server: Option<Arc<InspectorServer>>,
pub get_error_class_fn: Option<GetErrorClassFn>,
Expand Down Expand Up @@ -544,6 +546,7 @@ impl WebWorker {
let mut js_runtime = JsRuntime::new(RuntimeOptions {
module_loader: Some(options.module_loader.clone()),
startup_snapshot: options.startup_snapshot,
source_map_getter: options.source_map_getter,
get_error_class_fn: options.get_error_class_fn,
shared_array_buffer_store: options.shared_array_buffer_store.clone(),
compiled_wasm_module_store: options.compiled_wasm_module_store.clone(),
Expand Down
5 changes: 5 additions & 0 deletions runtime/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use deno_core::PollEventLoopOptions;
use deno_core::RuntimeOptions;
use deno_core::SharedArrayBufferStore;
use deno_core::SourceCodeCacheInfo;
use deno_core::SourceMapGetter;
use deno_cron::local::LocalCronHandler;
use deno_fs::FileSystem;
use deno_http::DefaultHttpPropertyExtractor;
Expand Down Expand Up @@ -161,6 +162,8 @@ pub struct WorkerOptions {
pub create_web_worker_cb: Arc<ops::worker_host::CreateWebWorkerCb>,
pub format_js_error_fn: Option<Arc<FormatJsErrorFn>>,

/// Source map reference for errors.
pub source_map_getter: Option<Rc<dyn SourceMapGetter>>,
pub maybe_inspector_server: Option<Arc<InspectorServer>>,
// If true, the worker will wait for inspector session and break on first
// statement of user code. Takes higher precedence than
Expand Down Expand Up @@ -223,6 +226,7 @@ impl Default for WorkerOptions {
origin_storage_dir: Default::default(),
cache_storage_dir: Default::default(),
broadcast_channel: Default::default(),
source_map_getter: Default::default(),
root_cert_store_provider: Default::default(),
node_resolver: Default::default(),
npm_resolver: Default::default(),
Expand Down Expand Up @@ -482,6 +486,7 @@ impl MainWorker {
module_loader: Some(options.module_loader.clone()),
startup_snapshot: options.startup_snapshot,
create_params: options.create_params,
source_map_getter: options.source_map_getter,
skip_op_registration: options.skip_op_registration,
get_error_class_fn: options.get_error_class_fn,
shared_array_buffer_store: options.shared_array_buffer_store.clone(),
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/compile_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ fn standalone_error() {
// On Windows, we cannot assert the file path (because '\').
// Instead we just check for relevant output.
assert_contains!(stderr, "error: Uncaught (in promise) Error: boom!");
assert_contains!(stderr, "throw new Error(\"boom!\");");
assert_contains!(stderr, "\n at boom (file://");
assert_contains!(stderr, "standalone_error.ts:2:9");
assert_contains!(stderr, "at foo (file://");
Expand Down Expand Up @@ -146,6 +147,7 @@ fn standalone_error_module_with_imports() {
// On Windows, we cannot assert the file path (because '\').
// Instead we just check for relevant output.
assert_contains!(stderr, "error: Uncaught (in promise) Error: boom!");
assert_contains!(stderr, "throw new Error(\"boom!\");");
assert_contains!(stderr, "\n at file://");
assert_contains!(stderr, "standalone_error_module_with_imports_2.ts:2:7");
output.assert_exit_code(1);
Expand Down

0 comments on commit 95847f4

Please sign in to comment.