From 26733971228e5c82ab84eac0ebc3764a06759bbb Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Wed, 16 Oct 2024 15:05:54 +0000
Subject: [PATCH] fix(isolated_declarations): fix potential memory leak (#6622)

`Scope` contains 2 x `FxHashMap`s, which own data outside of the arena. So this data will not be dropped when the allocator is dropped.

The scope for this becoming a memory leak in practice is limited for 2 reasons:

1. All `Scope`s except the root one are popped from the stack by the end of traversal. That last scope's hashmaps are always empty, unless there are unresolved references (references to globals).

2. `oxc_allocator::Vec` is currently `Drop`.

However, `oxc_allocator::Vec` will cease to be `Drop` in future, at which point this would become a real memory leak.

Additionally, it doesn't make sense to store temporary data in the arena, as the arena is intended to hold data that needs to live as long as the AST, which temporary data doesn't.
---
 crates/oxc_isolated_declarations/src/lib.rs   |  2 +-
 crates/oxc_isolated_declarations/src/scope.rs | 12 +++++-------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/crates/oxc_isolated_declarations/src/lib.rs b/crates/oxc_isolated_declarations/src/lib.rs
index 84303623efda5..89fdf38fee2d3 100644
--- a/crates/oxc_isolated_declarations/src/lib.rs
+++ b/crates/oxc_isolated_declarations/src/lib.rs
@@ -71,7 +71,7 @@ impl<'a> IsolatedDeclarations<'a> {
             ast: AstBuilder::new(allocator),
             strip_internal,
             internal_annotations: FxHashSet::default(),
-            scope: ScopeTree::new(allocator),
+            scope: ScopeTree::new(),
             errors: RefCell::new(vec![]),
         }
     }
diff --git a/crates/oxc_isolated_declarations/src/scope.rs b/crates/oxc_isolated_declarations/src/scope.rs
index 8f0fbb837404f..cc7dde16896f9 100644
--- a/crates/oxc_isolated_declarations/src/scope.rs
+++ b/crates/oxc_isolated_declarations/src/scope.rs
@@ -1,15 +1,14 @@
 use std::cell::Cell;
 
 use bitflags::bitflags;
-use oxc_allocator::{Allocator, Vec};
+use rustc_hash::FxHashMap;
+
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::ast::*;
-use oxc_ast::AstBuilder;
 #[allow(clippy::wildcard_imports)]
 use oxc_ast::{visit::walk::*, Visit};
 use oxc_span::Atom;
 use oxc_syntax::scope::{ScopeFlags, ScopeId};
-use rustc_hash::FxHashMap;
 
 bitflags! {
     #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -37,13 +36,12 @@ impl<'a> Scope<'a> {
 /// Linear tree of declaration scopes.
 #[derive(Debug)]
 pub struct ScopeTree<'a> {
-    levels: Vec<'a, Scope<'a>>,
+    levels: Vec<Scope<'a>>,
 }
 
 impl<'a> ScopeTree<'a> {
-    pub fn new(allocator: &'a Allocator) -> Self {
-        let ast = AstBuilder::new(allocator);
-        let levels = ast.vec1(Scope::new(ScopeFlags::Top));
+    pub fn new() -> Self {
+        let levels = vec![Scope::new(ScopeFlags::Top)];
         Self { levels }
     }