Skip to content

Commit 8bbda00

Browse files
authored
Merge pull request #13 from dak2/fix-memory-leak
Fix memory leak
2 parents d6c9104 + 920734c commit 8bbda00

7 files changed

Lines changed: 139 additions & 62 deletions

File tree

ext/src/lib.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ use magnus::{function, method, prelude::*, Error, Ruby};
66
use methodray_core::{
77
analyzer::AstInstaller,
88
env::{GlobalEnv, LocalEnv},
9-
parser, rbs,
9+
parser::ParseSession,
10+
rbs,
1011
};
1112

1213
#[magnus::wrap(class = "MethodRay::Analyzer")]
@@ -27,11 +28,11 @@ impl Analyzer {
2728
/// Execute type inference
2829
fn infer_types(&self, source: String) -> Result<String, Error> {
2930
// Parse
30-
let parse_result =
31-
parser::parse_ruby_source(&source, "source.rb".to_string()).map_err(|e| {
32-
let ruby = unsafe { Ruby::get_unchecked() };
33-
Error::new(ruby.exception_runtime_error(), e.to_string())
34-
})?;
31+
let session = ParseSession::new();
32+
let parse_result = session.parse_source(&source, "source.rb").map_err(|e| {
33+
let ruby = unsafe { Ruby::get_unchecked() };
34+
Error::new(ruby.exception_runtime_error(), e.to_string())
35+
})?;
3536

3637
// Build graph
3738
let mut genv = GlobalEnv::new();

rust/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ path = "src/lsp/main.rs"
1818
required-features = ["lsp"]
1919

2020
[dependencies]
21+
bumpalo = "3.14"
2122
smallvec = "1.13"
2223
rayon = "1.10"
2324
walkdir = "2.5"

rust/src/analyzer/definitions.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ fn extract_constant_path(node: &Node) -> Option<String> {
7979
#[cfg(test)]
8080
mod tests {
8181
use super::*;
82-
use crate::parser::parse_ruby_source;
82+
use crate::parser::ParseSession;
8383

8484
#[test]
8585
fn test_enter_exit_class_scope() {
@@ -150,7 +150,8 @@ mod tests {
150150
#[test]
151151
fn test_extract_simple_class_name() {
152152
let source = "class User; end";
153-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
153+
let session = ParseSession::new();
154+
let parse_result = session.parse_source(source, "test.rb").unwrap();
154155
let root = parse_result.node();
155156
let program = root.as_program_node().unwrap();
156157
let stmt = program.statements().body().first().unwrap();
@@ -163,7 +164,8 @@ mod tests {
163164
#[test]
164165
fn test_extract_qualified_class_name() {
165166
let source = "class Api::User; end";
166-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
167+
let session = ParseSession::new();
168+
let parse_result = session.parse_source(source, "test.rb").unwrap();
167169
let root = parse_result.node();
168170
let program = root.as_program_node().unwrap();
169171
let stmt = program.statements().body().first().unwrap();
@@ -176,7 +178,8 @@ mod tests {
176178
#[test]
177179
fn test_extract_deeply_qualified_class_name() {
178180
let source = "class Api::V1::Admin::User; end";
179-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
181+
let session = ParseSession::new();
182+
let parse_result = session.parse_source(source, "test.rb").unwrap();
180183
let root = parse_result.node();
181184
let program = root.as_program_node().unwrap();
182185
let stmt = program.statements().body().first().unwrap();
@@ -189,7 +192,8 @@ mod tests {
189192
#[test]
190193
fn test_extract_simple_module_name() {
191194
let source = "module Utils; end";
192-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
195+
let session = ParseSession::new();
196+
let parse_result = session.parse_source(source, "test.rb").unwrap();
193197
let root = parse_result.node();
194198
let program = root.as_program_node().unwrap();
195199
let stmt = program.statements().body().first().unwrap();
@@ -202,7 +206,8 @@ mod tests {
202206
#[test]
203207
fn test_extract_qualified_module_name() {
204208
let source = "module Api::V1; end";
205-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
209+
let session = ParseSession::new();
210+
let parse_result = session.parse_source(source, "test.rb").unwrap();
206211
let root = parse_result.node();
207212
let program = root.as_program_node().unwrap();
208213
let stmt = program.statements().body().first().unwrap();

rust/src/analyzer/install.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -536,13 +536,14 @@ impl<'a> AstInstaller<'a> {
536536
#[cfg(test)]
537537
mod tests {
538538
use super::*;
539-
use crate::parser::parse_ruby_source;
539+
use crate::parser::ParseSession;
540540
use crate::types::Type;
541541

542542
#[test]
543543
fn test_install_literal() {
544544
let source = r#"x = "hello""#;
545-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
545+
let session = ParseSession::new();
546+
let parse_result = session.parse_source(source, "test.rb").unwrap();
546547

547548
let mut genv = GlobalEnv::new();
548549
let mut lenv = LocalEnv::new();
@@ -568,7 +569,8 @@ mod tests {
568569
x = "hello"
569570
y = 42
570571
"#;
571-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
572+
let session = ParseSession::new();
573+
let parse_result = session.parse_source(source, "test.rb").unwrap();
572574

573575
let mut genv = GlobalEnv::new();
574576
let mut lenv = LocalEnv::new();
@@ -597,7 +599,8 @@ y = 42
597599
x = "hello"
598600
y = x.upcase
599601
"#;
600-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
602+
let session = ParseSession::new();
603+
let parse_result = session.parse_source(source, "test.rb").unwrap();
601604

602605
let mut genv = GlobalEnv::new();
603606
genv.register_builtin_method(Type::string(), "upcase", Type::string());
@@ -631,7 +634,8 @@ module Utils
631634
end
632635
end
633636
"#;
634-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
637+
let session = ParseSession::new();
638+
let parse_result = session.parse_source(source, "test.rb").unwrap();
635639

636640
let mut genv = GlobalEnv::new();
637641
let mut lenv = LocalEnv::new();
@@ -663,7 +667,8 @@ module Api
663667
end
664668
end
665669
"#;
666-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
670+
let session = ParseSession::new();
671+
let parse_result = session.parse_source(source, "test.rb").unwrap();
667672

668673
let mut genv = GlobalEnv::new();
669674
let mut lenv = LocalEnv::new();

rust/src/analyzer/tests/integration_test.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@
88
99
use crate::analyzer::AstInstaller;
1010
use crate::env::{GlobalEnv, LocalEnv};
11-
use crate::parser::parse_ruby_source;
11+
use crate::parser::ParseSession;
1212
use crate::types::Type;
1313

1414
/// Helper to run analysis on Ruby source code
1515
fn analyze(source: &str) -> (GlobalEnv, LocalEnv) {
16-
let parse_result = parse_ruby_source(source, "test.rb".to_string()).unwrap();
16+
let session = ParseSession::new();
17+
let parse_result = session.parse_source(source, "test.rb").unwrap();
1718

1819
let mut genv = GlobalEnv::new();
1920

rust/src/checker.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::analyzer::AstInstaller;
22
use crate::diagnostics::Diagnostic;
33
use crate::env::{GlobalEnv, LocalEnv};
4-
use crate::parser;
4+
use crate::parser::ParseSession;
55
use anyhow::{Context, Result};
66
use std::path::Path;
77

@@ -30,8 +30,12 @@ impl FileChecker {
3030
let source = std::fs::read_to_string(file_path)
3131
.with_context(|| format!("Failed to read {}", file_path.display()))?;
3232

33+
// Create ParseSession (arena allocator) for this check
34+
let session = ParseSession::new();
35+
3336
// Parse file
34-
let parse_result = parser::parse_ruby_file(file_path)
37+
let parse_result = session
38+
.parse_source(&source, &file_path.to_string_lossy())
3539
.with_context(|| format!("Failed to parse {}", file_path.display()))?;
3640

3741
// Create fresh GlobalEnv for this analysis
@@ -56,7 +60,7 @@ impl FileChecker {
5660
let diagnostics = collect_diagnostics(&genv, file_path);
5761

5862
Ok(diagnostics)
59-
}
63+
} // session drops here, freeing arena memory
6064
}
6165

6266
/// Load RBS methods from cache (CLI mode without Ruby runtime)

rust/src/parser.rs

Lines changed: 99 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,46 +1,81 @@
11
use anyhow::{Context, Result};
2+
use bumpalo::Bump;
23
use ruby_prism::{parse, ParseResult};
34
use std::fs;
45
use std::path::Path;
56

6-
/// Parse Ruby source code and return ruby-prism AST
7+
/// Parse session - manages source bytes for multiple files using arena allocation
78
///
8-
/// Note: Uses Box::leak internally to ensure 'static lifetime
9-
pub fn parse_ruby_file(file_path: &Path) -> Result<ParseResult<'static>> {
10-
let source = fs::read_to_string(file_path)
11-
.with_context(|| format!("Failed to read file: {}", file_path.display()))?;
12-
13-
parse_ruby_source(&source, file_path.to_string_lossy().to_string())
9+
/// Uses an arena allocator to efficiently manage source bytes during parsing.
10+
/// When the session is dropped, all memory is released at once.
11+
pub struct ParseSession {
12+
arena: Bump,
1413
}
1514

16-
/// Parse Ruby source code string
17-
pub fn parse_ruby_source(source: &str, file_name: String) -> Result<ParseResult<'static>> {
18-
// ruby-prism accepts &[u8]
19-
// Use Box::leak to ensure 'static lifetime (memory leak is acceptable for analysis tools)
20-
let source_bytes: &'static [u8] = Box::leak(source.as_bytes().to_vec().into_boxed_slice());
21-
let parse_result = parse(source_bytes);
22-
23-
// Check parse errors
24-
let error_messages: Vec<String> = parse_result
25-
.errors()
26-
.map(|e| {
27-
format!(
28-
"Parse error at offset {}: {}",
29-
e.location().start_offset(),
30-
e.message()
31-
)
32-
})
33-
.collect();
34-
35-
if !error_messages.is_empty() {
36-
anyhow::bail!(
37-
"Failed to parse Ruby source in {}:\n{}",
38-
file_name,
39-
error_messages.join("\n")
40-
);
15+
impl ParseSession {
16+
pub fn new() -> Self {
17+
Self { arena: Bump::new() }
18+
}
19+
20+
/// Create with pre-allocated capacity (recommended for batch file processing)
21+
pub fn with_capacity(capacity: usize) -> Self {
22+
Self {
23+
arena: Bump::with_capacity(capacity),
24+
}
25+
}
26+
27+
/// Allocate source in arena and parse
28+
pub fn parse_source<'a>(&'a self, source: &str, file_name: &str) -> Result<ParseResult<'a>> {
29+
// Copy bytes to arena
30+
let source_bytes = self.arena.alloc_slice_copy(source.as_bytes());
31+
let parse_result = parse(source_bytes);
32+
33+
// Check for parse errors
34+
let error_messages: Vec<String> = parse_result
35+
.errors()
36+
.map(|e| {
37+
format!(
38+
"Parse error at offset {}: {}",
39+
e.location().start_offset(),
40+
e.message()
41+
)
42+
})
43+
.collect();
44+
45+
if !error_messages.is_empty() {
46+
anyhow::bail!(
47+
"Failed to parse Ruby source in {}:\n{}",
48+
file_name,
49+
error_messages.join("\n")
50+
);
51+
}
52+
53+
Ok(parse_result)
54+
}
55+
56+
/// Read file and parse
57+
pub fn parse_file<'a>(&'a self, file_path: &Path) -> Result<ParseResult<'a>> {
58+
let source = fs::read_to_string(file_path)
59+
.with_context(|| format!("Failed to read file: {}", file_path.display()))?;
60+
61+
self.parse_source(&source, &file_path.to_string_lossy())
4162
}
4263

43-
Ok(parse_result)
64+
/// Get allocated memory size (for debugging)
65+
pub fn allocated_bytes(&self) -> usize {
66+
self.arena.allocated_bytes()
67+
}
68+
69+
/// Reset arena (for memory control during batch file processing)
70+
pub fn reset(&mut self) {
71+
self.arena.reset();
72+
}
73+
}
74+
75+
impl Default for ParseSession {
76+
fn default() -> Self {
77+
Self::new()
78+
}
4479
}
4580

4681
#[cfg(test)]
@@ -51,21 +86,24 @@ mod tests {
5186
fn test_parse_simple_ruby() {
5287
let source = r#"x = 1
5388
puts x"#;
54-
let result = parse_ruby_source(source, "test.rb".to_string());
89+
let session = ParseSession::new();
90+
let result = session.parse_source(source, "test.rb");
5591
assert!(result.is_ok());
5692
}
5793

5894
#[test]
5995
fn test_parse_string_literal() {
6096
let source = r#""hello".upcase"#;
61-
let result = parse_ruby_source(source, "test.rb".to_string());
97+
let session = ParseSession::new();
98+
let result = session.parse_source(source, "test.rb");
6299
assert!(result.is_ok());
63100
}
64101

65102
#[test]
66103
fn test_parse_array_literal() {
67104
let source = r#"[1, 2, 3].map { |x| x * 2 }"#;
68-
let result = parse_ruby_source(source, "test.rb".to_string());
105+
let session = ParseSession::new();
106+
let result = session.parse_source(source, "test.rb");
69107
assert!(result.is_ok());
70108
}
71109

@@ -75,22 +113,44 @@ puts x"#;
75113
x = "hello"
76114
x.upcase
77115
end"#;
78-
let result = parse_ruby_source(source, "test.rb".to_string());
116+
let session = ParseSession::new();
117+
let result = session.parse_source(source, "test.rb");
79118
assert!(result.is_ok());
80119
}
81120

82121
#[test]
83122
fn test_parse_invalid_ruby() {
84123
let source = "def\nend end";
85-
let result = parse_ruby_source(source, "test.rb".to_string());
124+
let session = ParseSession::new();
125+
let result = session.parse_source(source, "test.rb");
86126
assert!(result.is_err());
87127
}
88128

89129
#[test]
90130
fn test_parse_method_call() {
91131
let source = r#"user = User.new
92132
user.save"#;
93-
let result = parse_ruby_source(source, "test.rb".to_string());
133+
let session = ParseSession::new();
134+
let result = session.parse_source(source, "test.rb");
94135
assert!(result.is_ok());
95136
}
137+
138+
#[test]
139+
fn test_parse_session_memory_tracking() {
140+
let session = ParseSession::new();
141+
let source = "x = 1";
142+
let _ = session.parse_source(source, "test.rb").unwrap();
143+
assert!(session.allocated_bytes() > 0);
144+
}
145+
146+
#[test]
147+
fn test_parse_session_reset() {
148+
let mut session = ParseSession::new();
149+
let source = "x = 1";
150+
let _ = session.parse_source(source, "test.rb").unwrap();
151+
let before_reset = session.allocated_bytes();
152+
session.reset();
153+
// After reset, allocated_bytes may still report used chunks but internal data is cleared
154+
assert!(before_reset > 0);
155+
}
96156
}

0 commit comments

Comments
 (0)