From de990fa7c7840a1ac964a48f00aff1358eaec562 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 17 Oct 2016 22:19:54 +0200 Subject: [PATCH] BUG-6757: revert fix for BUG-4456 The fix has introduced a massive memory leak, which causes all of temporary build objects to be retained in the final SchemaContext. Instead of the leak, add an explicit guard to detect extensions (transitively) referencing themselves. Change-Id: If90a4f9420866a6392ce97c71837915fa41ae0c8 Signed-off-by: Robert Varga (cherry picked from commit e5226bf4ddd7a42aa13c392b2fc4d02d27ae2f74) (cherry picked from commit 91f43b1ce6453fef9e04e6673e7637fb5dba8b38) --- .../stmt/rfc6020/ExtensionStatementImpl.java | 35 ++++++++++++---- .../AbstractEffectiveDocumentedNode.java | 14 +------ .../effective/AbstractEffectiveModule.java | 5 +-- .../DeclaredEffectiveStatementBase.java | 16 ++------ .../effective/EffectiveStatementBase.java | 40 ++----------------- .../ExtensionEffectiveStatementImpl.java | 31 +++++--------- .../yangtools/yang/stmt/test/Bug4456Test.java | 2 +- 7 files changed, 49 insertions(+), 94 deletions(-) diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java index 0dd010deba..74c6e67cb2 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/ExtensionStatementImpl.java @@ -7,6 +7,8 @@ */ package org.opendaylight.yangtools.yang.parser.stmt.rfc6020; +import java.util.HashSet; +import java.util.Set; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.Rfc6020Mapping; import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; @@ -32,29 +34,48 @@ public class ExtensionStatementImpl extends AbstractDeclaredStatement imp .add(Rfc6020Mapping.STATUS, 0, 1) .build(); - protected ExtensionStatementImpl(StmtContext context) { + protected ExtensionStatementImpl(final StmtContext context) { super(context); } public static class Definition extends AbstractStatementSupport> { + private static final ThreadLocal>> BUILDING = new ThreadLocal<>(); public Definition() { super(Rfc6020Mapping.EXTENSION); } @Override - public QName parseArgumentValue(StmtContext ctx, String value) { + public QName parseArgumentValue(final StmtContext ctx, final String value) { return Utils.qNameFromArgument(ctx, value); } @Override - public ExtensionStatement createDeclared(StmtContext ctx) { + public ExtensionStatement createDeclared(final StmtContext ctx) { return new ExtensionStatementImpl(ctx); } @Override - public EffectiveStatement createEffective(StmtContext> ctx) { - return new ExtensionEffectiveStatementImpl(ctx); + public EffectiveStatement createEffective( + final StmtContext> ctx) { + Set> building = BUILDING.get(); + if (building == null) { + building = new HashSet<>(); + BUILDING.set(building); + } + + SourceException.throwIf(building.contains(ctx), ctx.getStatementSourceReference(), + "Extension %s references itself", ctx.getStatementArgument()); + + building.add(ctx); + try { + return new ExtensionEffectiveStatementImpl(ctx); + } finally { + building.remove(ctx); + if (building.isEmpty()) { + BUILDING.remove(); + } + } } @Override @@ -63,8 +84,8 @@ public class ExtensionStatementImpl extends AbstractDeclaredStatement imp } @Override - public void onFullDefinitionDeclared(StmtContext.Mutable> stmt) throws SourceException { + public void onFullDefinitionDeclared(final StmtContext.Mutable> stmt) { super.onFullDefinitionDeclared(stmt); SUBSTATEMENT_VALIDATOR.validate(stmt); } diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java index 8c665d7a0a..39a7adabc2 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveDocumentedNode.java @@ -19,24 +19,14 @@ abstract class AbstractEffectiveDocumentedNode private final String reference; private final Status status; - AbstractEffectiveDocumentedNode(final StmtContext ctx) { - this(ctx, true); - } - /** * Constructor. * * @param ctx * context of statement. - * @param buildUnknownSubstatements - * if it is false, the unknown substatements are omitted from - * build of effective substatements till the call of either - * effectiveSubstatements or getOmittedUnknownSubstatements - * method of EffectiveStatementBase class. The main purpose of - * this is to allow the build of recursive extension definitions. */ - AbstractEffectiveDocumentedNode(final StmtContext ctx, boolean buildUnknownSubstatements) { - super(ctx, buildUnknownSubstatements); + protected AbstractEffectiveDocumentedNode(final StmtContext ctx) { + super(ctx); DescriptionEffectiveStatementImpl descStmt = firstEffective(DescriptionEffectiveStatementImpl.class); if (descStmt != null) { diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java index 883b54cd5c..fd60af3a1b 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/AbstractEffectiveModule.java @@ -172,10 +172,7 @@ abstract class AbstractEffectiveModule> exte featuresInit.add((FeatureDefinition) effectiveStatement); } if (effectiveStatement instanceof ExtensionEffectiveStatementImpl) { - ExtensionEffectiveStatementImpl extensionDefinition = (ExtensionEffectiveStatementImpl) effectiveStatement; - extensionDefinition.initUnknownSchemaNodes(); - extensionNodesInit - .add(extensionDefinition); + extensionNodesInit.add((ExtensionEffectiveStatementImpl) effectiveStatement); } if (effectiveStatement instanceof DataSchemaNode) { DataSchemaNode dataSchemaNode = (DataSchemaNode) effectiveStatement; diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java index 212a65e813..e414a46871 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/DeclaredEffectiveStatementBase.java @@ -21,24 +21,14 @@ public abstract class DeclaredEffectiveStatementBase ctx) { - this(ctx, true); - } - /** * Constructor. * * @param ctx * context of statement. - * @param buildUnknownSubstatements - * if it is false, the unknown substatements are omitted from - * build of effective substatements till the call of either - * effectiveSubstatements or getOmittedUnknownSubstatements - * method of EffectiveStatementBase class. The main purpose of - * this is to allow the build of recursive extension definitions. */ - protected DeclaredEffectiveStatementBase(StmtContext ctx, final boolean buildUnknownSubstatements) { - super(ctx, buildUnknownSubstatements); + protected DeclaredEffectiveStatementBase(StmtContext ctx) { + super(ctx); this.argument = ctx.getStatementArgument(); this.statementSource = ctx.getStatementSource(); @@ -75,4 +65,4 @@ public abstract class DeclaredEffectiveStatementBase> }; private final List> substatements; - private final List> unknownSubstatementsToBuild; - - protected EffectiveStatementBase(final StmtContext ctx) { - this(ctx, true); - } /** * Constructor. * * @param ctx * context of statement. - * @param buildUnknownSubstatements - * if it is false, the unknown substatements are omitted from - * build of effective substatements till the call of either - * effectiveSubstatements or getOmittedUnknownSubstatements - * method. The main purpose of this is to allow the build of - * recursive extension definitions. */ - protected EffectiveStatementBase(final StmtContext ctx, boolean buildUnknownSubstatements) { - + protected EffectiveStatementBase(final StmtContext ctx) { final Collection> effectiveSubstatements = ctx.effectiveSubstatements(); final Collection> substatementsInit = new ArrayList<>(); @@ -80,24 +68,8 @@ public abstract class EffectiveStatementBase> } substatementsInit.addAll(effectiveSubstatements); - Collection> substatementsToBuild = Collections2.filter(substatementsInit, - IS_SUPPORTED_TO_BUILD_EFFECTIVE); - if (!buildUnknownSubstatements) { - this.unknownSubstatementsToBuild = ImmutableList.copyOf(Collections2.filter(substatementsToBuild, - IS_UNKNOWN_STATEMENT_CONTEXT)); - substatementsToBuild = Collections2.filter(substatementsToBuild, - Predicates.not(IS_UNKNOWN_STATEMENT_CONTEXT)); - } else { - this.unknownSubstatementsToBuild = ImmutableList.of(); - } - - this.substatements = ImmutableList.copyOf(Collections2.transform(substatementsToBuild, - StmtContextUtils.buildEffective())); - } - - Collection> getOmittedUnknownSubstatements() { - return Collections2.transform(unknownSubstatementsToBuild, - StmtContextUtils.buildEffective()); + this.substatements = ImmutableList.copyOf(Collections2.transform(Collections2.filter(substatementsInit, + IS_SUPPORTED_TO_BUILD_EFFECTIVE), StmtContextUtils.buildEffective())); } @Override @@ -112,11 +84,7 @@ public abstract class EffectiveStatementBase> @Override public final Collection> effectiveSubstatements() { - if (unknownSubstatementsToBuild.isEmpty()) { - return substatements; - } else { - return ImmutableList.copyOf(Iterables.concat(substatements, getOmittedUnknownSubstatements())); - } + return substatements; } protected final > S firstEffective(final Class type) { diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java index be76294246..4a13515b8d 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/ExtensionEffectiveStatementImpl.java @@ -9,7 +9,6 @@ package org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective; import com.google.common.collect.ImmutableList; import java.util.ArrayList; -import java.util.Collection; import java.util.List; import java.util.Objects; import org.opendaylight.yangtools.yang.common.QName; @@ -26,15 +25,23 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented private final String argument; private final SchemaPath schemaPath; - private List unknownNodes; + private final List unknownNodes; private final boolean yin; public ExtensionEffectiveStatementImpl( final StmtContext> ctx) { - super(ctx, false); + super(ctx); this.qname = ctx.getStatementArgument(); this.schemaPath = ctx.getSchemaPath().get(); + final List unknownNodesInit = new ArrayList<>(); + for (EffectiveStatement unknownNode : effectiveSubstatements()) { + if (unknownNode instanceof UnknownSchemaNode) { + unknownNodesInit.add((UnknownSchemaNode) unknownNode); + } + } + this.unknownNodes = ImmutableList.copyOf(unknownNodesInit); + // initFields ArgumentEffectiveStatementImpl argumentSubstatement = firstEffective(ArgumentEffectiveStatementImpl.class); if (argumentSubstatement != null) { @@ -53,21 +60,6 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented } } - void initUnknownSchemaNodes() { - if (unknownNodes != null) { - return; - } - - Collection> buildedUnknownNodes = getOmittedUnknownSubstatements(); - List unknownNodesInit = new ArrayList<>(); - for (EffectiveStatement unknownNode : buildedUnknownNodes) { - if (unknownNode instanceof UnknownSchemaNode) { - unknownNodesInit.add((UnknownSchemaNode) unknownNode); - } - } - this.unknownNodes = ImmutableList.copyOf(unknownNodesInit); - } - @Override public QName getQName() { return qname; @@ -80,9 +72,6 @@ public class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumented @Override public List getUnknownSchemaNodes() { - if(unknownNodes == null) { - initUnknownSchemaNodes(); - } return unknownNodes; } diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/test/Bug4456Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/test/Bug4456Test.java index 88eb8cdba4..5af276c546 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/test/Bug4456Test.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/test/Bug4456Test.java @@ -25,7 +25,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; public class Bug4456Test { - @Test + @Test(expected=SourceException.class) public void test() throws IOException, URISyntaxException, SourceException, ReactorException { SchemaContext schema = StmtTestUtils.parseYangSources("/bugs/bug4456"); assertNotNull(schema); -- 2.36.6