This is an automated email from the ASF dual-hosted git repository.
joshtynjala pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
The following commit(s) were added to refs/heads/develop by this push:
new 35eed62f1 FunctionScope: fix exception where a FunctionNode has been
garbage collected, and a new FunctionNode gets re-attached to the existing
FunctionScope scope
35eed62f1 is described below
commit 35eed62f13519c659e6346d26cca3f44afe3170f
Author: Josh Tynjala <[email protected]>
AuthorDate: Thu Apr 3 15:29:28 2025 -0700
FunctionScope: fix exception where a FunctionNode has been garbage
collected, and a new FunctionNode gets re-attached to the existing
FunctionScope scope
Similar to FunctionNode's discardFunctionBody() method, we need to clear
out any old definitions that will no longer be valid because the new function
body (even if it has the same contents) will parse new VariableNodes and things
for local variables and create new definitions to replace the old ones.
This issue is definitely an edge case, but I've hit it more than once over
the years. It seems to be uncommon for FunctionNodes to get garbage collected,
but when they do, an assertion failure in parseFunctionBody() gets triggered.
In particular, this was causing the ASDoc JSON generation to fail
intermittently (but relatively often, since it parses so many classes). It was
originally doing that silently (so files weren't generated, but the ASDoc tool
reported success), but I fixed th [...]
In the past, I think that I encountered this same issue when implementing
the -watch compiler option. When watching, the compiler continues to run after
the initial compilation and then recompiles on source path file changes.
Running indefinitely made it more likely for the GC to eventually run and
FunctionNodes to be replaced. I found a workaround for that at the time (I
think using workspace.startBuilding() and workspace.doneBuilding()), but that
didn't seem to work here. That make [...]
Removing definitions like discardFunctionBody() when the FunctionScope
detects that function body hasn't been parsed yet seems like it will be much
more reliable.
The reason that FunctionNodes can be GCed is that a weak reference to the
AST is kept by ASCompilationUnit's syntax tree request. At first, it holds a
hard reference, but at some point later in the compilation process, it is
deemed safe to be GCed. From that point, it is demoted to a weak reference.
This appears to be by design in the code that was donated to us by Adobe. This
process of demoting from strong to weak reference technically allows the
FunctionNode (technically, I think i [...]
I'm not sure if FunctionScope is the best place to put this code. There may
be a way to put it into FunctionNode instead, which would keep the quirks of
FunctionNodes being able to defer parsing bodies or to discard bodies a bit
more encapsulated. However, even in FunctionScope, that's still pretty specific
to functions.
---
.../compiler/internal/scopes/FunctionScope.java | 46 ++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git
a/compiler/src/main/java/org/apache/royale/compiler/internal/scopes/FunctionScope.java
b/compiler/src/main/java/org/apache/royale/compiler/internal/scopes/FunctionScope.java
index c6ccbf297..b734781f3 100644
---
a/compiler/src/main/java/org/apache/royale/compiler/internal/scopes/FunctionScope.java
+++
b/compiler/src/main/java/org/apache/royale/compiler/internal/scopes/FunctionScope.java
@@ -19,7 +19,14 @@
package org.apache.royale.compiler.internal.scopes;
+import java.util.Collection;
+
+import org.apache.royale.compiler.definitions.IDefinition;
+import org.apache.royale.compiler.definitions.IParameterDefinition;
+import org.apache.royale.compiler.internal.tree.as.FunctionNode;
import org.apache.royale.compiler.internal.tree.as.ScopedBlockNode;
+import org.apache.royale.compiler.tree.as.IASNode;
+import org.apache.royale.compiler.tree.as.IScopedNode;
/**
* Sub-class of ASScope used for function scopes. all definition in scope of
@@ -38,4 +45,43 @@ public class FunctionScope extends ASScope
super(containingScope);
}
+ @Override
+ public void reconnectScopeNode(IScopedNode node)
+ {
+ IASNode parentNode = node.getParent();
+ if (parentNode instanceof FunctionNode)
+ {
+ FunctionNode functionNode = (FunctionNode) parentNode;
+ if (!functionNode.hasBeenParsed())
+ {
+ // the compiler holds weak references to nodes in certain
places
+ // which allows them to be garbage collected. however, from
time
+ // to time, the compiler may need the node again later, after
it
+ // has been garbage collected, so that node needs to be
+ // recreated.
+ // when a function node is recreated, its scope may still be
+ // populated with definitions from the old function node's
body,
+ // such as local variables. upon recreation, those local
+ // definitions should be considered invalid because they will
+ // be replaced with new definitions after parsing the new
+ // function node's body.
+ // to reiterate, removing the definitions below is not the same
+ // case where a function node's discardFunctionBody() method is
+ // called and the local definitions need to be removed.
instead,
+ // this is a separate case where a function node is garbage
+ // collected and the body was never discarded, so the scope
+ // still contains definitions from the old function node.
+ Collection<IDefinition> localDefs = getAllLocalDefinitions();
+ for (IDefinition def : localDefs)
+ {
+ if (! (def instanceof IParameterDefinition))
+ {
+ removeDefinition(def);
+ }
+ }
+ }
+ }
+ super.reconnectScopeNode(node);
+ }
+
}
