Skip to content

Fix leak in closures with circular references - #695

Merged
SuperFola merged 3 commits into
devfrom
fix/leak
Jul 7, 2026
Merged

Fix leak in closures with circular references#695
SuperFola merged 3 commits into
devfrom
fix/leak

Conversation

@SuperFola

@SuperFolaSuperFola commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown
Member

Description

Closes # (issue)

Checklist

  • I have read the Contributor guide
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation if needed (on https://github.com/ArkScript-lang/website, content/docs/)
  • I have added tests that prove my fix/feature is working
  • New and existing tests pass locally with my changes
  • I confirm that I am the author of this code and release it to the ArkScript project under the MPL-2.0 license. This contribution does not contain code from other sources, including code generated by a Large Language Model ("AI").

@github-actions

github-actionsBot commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown

Static analysis report

Lizard report

Listing only functions with cyclomatic complexity >= 15 or NLOC >= 100 or parameters >= 6.

FilenameStart line:end lineFunction nameParametersNLOCCCN
src/arkreactor/VM/VM.cpp450:2153Ark::VM::unsafeRun21432281
src/arkreactor/VM/VM.cpp2307:2401Ark::VM::backtrace38120
src/arkreactor/VM/VM.cpp2165:2222Ark::VM::throwArityError44715
src/arkreactor/Builtins/Time.cpp72:92Ark::internal::Builtins::Time::makeTimestamp6205
Report about files you didn't modify in this PR
FilenameStart line:end lineFunction nameParametersNLOCCCN
src/arkreactor/Compiler/Macros/Processor.cpp259:643Ark::internal::MacroProcessor::evaluate3353119
src/arkreactor/Compiler/BytecodeReader.cpp385:596Ark::BytecodeReader::display417192
src/arkreactor/Error/Diagnostics.cpp45:192Ark::Diagnostics::makeContext410944
src/arkscript/main.cpp24:383main231043
src/arkreactor/Compiler/AST/Parser.cpp841:975Ark::internal::Parser::string113142
src/arkscript/JsonCompiler.cpp27:276JsonCompiler::_compile121438
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp163:268Ark::internal::NameResolutionPass::visitKeyword38535
src/arkreactor/Builtins/Slice.cpp21:93Ark::internal::Builtins::slice16334
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp355:434Ark::internal::ASTLowerer::compileListInstruction36432
src/arkreactor/Compiler/AST/Node.cpp179:285Ark::internal::Node::repr09030
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp853:999Ark::internal::ASTLowerer::handleFunctionCall311430
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp212:317Ark::internal::ASTLowerer::compileExpression58829
src/arkreactor/Compiler/Macros/Processor.cpp107:191Ark::internal::MacroProcessor::processNode36328
src/arkreactor/Compiler/AST/Node.cpp287:368Ark::internal::Node::debugPrint17026
src/arkreactor/Compiler/BytecodeReader.cpp612:713Ark::BytecodeReader::printInstruction79825
src/arkreactor/TypeChecker.cpp149:236Ark::types::generateError67525
src/arkreactor/TypeChecker.cpp32:147Ark::types::displayContract610224
src/arkreactor/Compiler/NameResolution/NameResolutionPass.cpp57:161Ark::internal::NameResolutionPass::visit28323
src/arkreactor/Compiler/AST/Parser.cpp289:417Ark::internal::Parser::import_19823
include/utf8.hpp138:184utf8::isValid14421
src/arkreactor/VM/Value/Value.cpp81:149Ark::Value::toString25421
src/arkreactor/Compiler/AST/Optimizer.cpp35:85Ark::internal::Optimizer::countAndPruneDeadCode14220
src/arkreactor/Compiler/NameResolution/StaticScope.cpp70:111Ark::internal::NamespaceScope::get33219
src/arkreactor/Compiler/AST/Optimizer.cpp87:144Ark::internal::Optimizer::pruneUnusedGlobalVariables15018
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp799:851Ark::internal::ASTLowerer::handleOperator34218
src/arkreactor/Builtins/String.cpp47:113fmt::formatter::parse15918
src/arkscript/Formatter.cpp212:274Formatter::format35918
src/arkscript/Formatter.cpp527:585Formatter::formatCall25018
src/arkreactor/Compiler/Macros/Executors/Function.cpp16:89Ark::internal::FunctionExecutor::applyMacro25617
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp107:129Ark::internal::ASTLowerer::nodeProducesOutput11817
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp597:645Ark::internal::ASTLowerer::compileLetMutSet44017
include/Ark/Compiler/AST/Predicates.hpp142:166Ark::internal::IsSymbol::operator ( )12416
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp514:595Ark::internal::ASTLowerer::compileFunction35916
src/arkscript/Formatter.cpp324:366Formatter::formatFunction23616
src/arkreactor/Compiler/Macros/Executors/Function.cpp99:156Ark::internal::FunctionExecutor::unify55015
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp22:260Ark::internal::IROptimizer::IROptimizer121914
src/arkreactor/Error/Diagnostics.cpp194:211Ark::Diagnostics::helper7172

CppCheck report

FilenameLineTypeDescription
include/Ark/VM/VM.inl267styleVariable 'maybe_value_ptr' can be declared as pointer to const
src/arkreactor/Builtins/Time.cpp32styleParameter 'n' can be declared as reference to const
src/arkreactor/VM/VM.cpp0informationToo many #ifdef configurations - cppcheck only checks 12 of 13 configurations. Use --force to check all configurations.
src/arkreactor/VM/VM.cpp281errorIterators of different containers 'm_execution_contexts.emplace_back(std::make_unique())' and 'm_execution_contexts.front()' are used together.
Report files about files you didn't modify in this PR
FilenameLineTypeDescription
src/arkreactor/Builtins/Bytecode.cpp23styleParameter 'vm' can be declared as pointer to const
src/arkreactor/Compiler/BytecodeReader.cpp20performanceVariable 'm_arg_kinds' is assigned in constructor body. Consider performing initialization in initialization list.
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp249styleConsider using std::transform algorithm instead of a raw loop.
src/arkreactor/Compiler/IntermediateRepresentation/IROptimizer.cpp256styleConsider using std::transform algorithm instead of a raw loop.
src/arkreactor/Compiler/Lowerer/ASTLowerer.cpp620styleConsider using std::count_if algorithm instead of a raw loop.
src/arkreactor/Compiler/Lowerer/LocalsLocator.cpp26styleThe scope of the variable 'data' can be reduced.
src/arkreactor/Compiler/NameResolution/ScopeResolver.cpp136styleConsider using std::find_if algorithm instead of a raw loop.
src/arkreactor/State.cpp183styleVariable 'bytecode_hash' is assigned a value that is never used.
src/arkreactor/VM/SharedLibrary.cpp0informationToo many #ifdef configurations - cppcheck only checks 12 of 13 configurations. Use --force to check all configurations.
include/Ark/VM/Value/Future.hpp50styleUnused private function: 'Future::deleteSelfViaVM'
src/arkreactor/VM/Value/Future.cpp23performanceVariable 'm_value' is assigned in constructor body. Consider performing initialization in initialization list.

@codspeed-hq

codspeed-hqBot commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 18 untouched benchmarks

Comparing fix/leak (8fc1120) with dev (a68b045)

Open in CodSpeed

@github-actions

github-actionsBot commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown

Super Instructions report

Some Super Instructions are under the usage threshold (24).

Super InstructionUses in compiled code
APPEND_IN_PLACE_SYM_INDEX23
INCREMENT19
DECREMENT18
STORE_FROM15
INCREMENT_BY_INDEX14
LT_CONST_JUMP_IF_FALSE9
NEQ_SYM_JUMP_IF_FALSE4
SET_VAL_TAIL4
STORE_HEAD2
MUL_BY2
CHECK_TYPE_OF2
NEQ_CONST_JUMP_IF_TRUE2
SET_VAL_TAIL_BY_INDEX1
SET_VAL_HEAD_BY_INDEX1
MUL_SET_VAL1
SET_VAL_HEAD1
STORE_TAIL1
Super Instructions over the threshold
Super InstructionUses in compiled code
CALL_SYMBOL6419
LOAD_CONST_LOAD_CONST5912
LOAD_CONST_STORE3165
LOAD_CONST_SET_VAL921
CALL_BUILTIN912
CALL_BUILTIN_WITHOUT_RETURN_ADDRESS655
INCREMENT_STORE619
AT_SYM_SYM571
GET_FIELD_FROM_SYMBOL_INDEX505
STORE_LIST385
GET_FIELD_FROM_SYMBOL342
APPEND_IN_PLACE_SYM341
LT_LEN_SYM_JUMP_IF_FALSE331
CALL_SYMBOL_BY_INDEX322
EQ_SYM_INDEX_JUMP_IF_TRUE219
SET_VAL_FROM_INDEX160
SET_VAL_FROM141
LT_SYM_JUMP_IF_FALSE130
STORE_FROM_INDEX127
DECREMENT_STORE120
STORE_LEN116
FUSED_MATH114
GT_CONST_JUMP_IF_FALSE112
DECREMENT_BY_INDEX97
GT_CONST_JUMP_IF_TRUE84
LT_CONST_JUMP_IF_TRUE83
AT_SYM_INDEX_SYM_INDEX83
MUL_BY_INDEX80
CALL_CURRENT_PAGE75
EQ_CONST_JUMP_IF_TRUE53
AT_SYM_INDEX_CONST51
GT_SYM_JUMP_IF_FALSE39
STORE_HEAD_BY_INDEX36
STORE_TAIL_BY_INDEX35
CHECK_TYPE_OF_BY_INDEX30

@github-actions

github-actionsBot commented Jun 25, 2026•
edited
Loading

Copy link
Copy Markdown

Fuzzing report

/usr/local/bin/afl-whatsup status check tool for afl-fuzz by Michal Zalewski

Summary stats

 Fuzzers alive : 0
Dead or remote : 1 (included in stats)
Total run time : 5 minutes, 0 seconds
Total execs : 38 thousands
Cumulative speed : 128 execs/sec
Pending items : 137 faves, 1153 total
Coverage reached : 8.59%
Crashes saved : 0
Hangs saved : 0

Cycles without finds : 0
Time without finds : 0

[+] Captured 44227 tuples (map size 270317, highest value 255, total values 429289444) in '/dev/null'.
[+] A coverage of 44227 edges were achieved out of 270336 existing (16.36%) with 1161 input files.

@coveralls

coveralls commented Jun 26, 2026•
edited
Loading

Copy link
Copy Markdown

Coverage Report for CI Build 28817801681

Warning

No base build found for commit a68b045 on dev.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 94.381%

Details

  • Patch coverage: 3 uncovered changes across 1 file (206 of 209 lines covered, 98.56%).

Uncovered Changes

FileChangedCovered%
include/Ark/VM/VM.inl20920698.56%

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats

Coverage Status
Relevant Lines:10927
Covered Lines:10313
Line Coverage:94.38%
Coverage Strength:991593.63 hits per line
💛 - Coveralls

SuperFola
SuperFola commented Jun 29, 2026
View reviewed changes
Comment threadinclude/Ark/VM/ExecutionContext.hpp Outdated
std::optional<ClosureScope> saved_scope {}; ///< Scope created by CAPTURE <x> instructions, used by the MAKE_CLOSURE instruction
std::optional<uint16_t> capture_rename_id {};
std::vector<std::shared_ptr<ClosureScope>> stacked_closure_scopes {}; ///< Stack the closure scopes to keep the closure alive as long as we are calling them
// todo: handle deletion of scopes

@SuperFolaSuperFolaJun 29, 2026

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not merge while in todo

@SuperFola
SuperFolaforce-pushed the fix/leak branch 7 times, most recently from 61bb298 to 389dda4CompareJuly 6, 2026 19:28
@SuperFola
SuperFola marked this pull request as ready for review July 7, 2026 04:31
@SuperFola
SuperFola merged commit 40d4620 into devJul 7, 2026
37 of 38 checks passed
@SuperFola
SuperFola deleted the fix/leak branch July 7, 2026 04:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SuperFola@coveralls