From: Robert Varga Date: Mon, 17 Oct 2016 20:19:54 +0000 (+0200) Subject: BUG-6757: revert fix for BUG-4456 X-Git-Tag: release/boron-sr1~38 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=91f43b1ce6453fef9e04e6673e7637fb5dba8b38;hp=0de136825605fc7c15b49bdb1841ff5ea48d8a21;p=yangtools.git 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) --- 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 062a3e90a7..9a09f2b4ab 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; @@ -20,6 +22,7 @@ import org.opendaylight.yangtools.yang.parser.spi.SubstatementValidator; import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractDeclaredStatement; import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractStatementSupport; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; +import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; import org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective.ExtensionEffectiveStatementImpl; public class ExtensionStatementImpl extends AbstractDeclaredStatement implements ExtensionStatement { @@ -31,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 @@ -62,7 +84,7 @@ public class ExtensionStatementImpl extends AbstractDeclaredStatement imp } @Override - public void onFullDefinitionDeclared(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 6ff829e21d..ce4eeff53b 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 @@ public abstract class AbstractEffectiveDocumentedNode 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 AbstractEffectiveDocumentedNode(final StmtContext ctx, boolean buildUnknownSubstatements) { - super(ctx, buildUnknownSubstatements); + protected AbstractEffectiveDocumentedNode(final StmtContext ctx) { + super(ctx); final 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 b4c8fe965c..db7aa9ebed 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 @@ -176,10 +176,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 e460303861..a1fdda30be 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(); diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java index 7f55295465..81374085f6 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/effective/EffectiveStatementBase.java @@ -8,7 +8,6 @@ package org.opendaylight.yangtools.yang.parser.stmt.rfc6020.effective; import com.google.common.base.Optional; -import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; @@ -27,32 +26,15 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; import org.opendaylight.yangtools.yang.parser.stmt.reactor.StatementContextBase; public abstract class EffectiveStatementBase> implements EffectiveStatement { - - private static final Predicate> IS_UNKNOWN_STATEMENT_CONTEXT = - StmtContextUtils::isUnknownStatement; - private static final Predicate> IS_NOT_UNKNOWN_STATEMENT_CONTEXT = - Predicates.not(IS_UNKNOWN_STATEMENT_CONTEXT); - 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, final boolean buildUnknownSubstatements) { + protected EffectiveStatementBase(final StmtContext ctx) { final Collection> effectiveSubstatements = ctx.effectiveSubstatements(); final Collection> substatementsInit = new ArrayList<>(); @@ -71,21 +53,8 @@ public abstract class EffectiveStatementBase> } substatementsInit.addAll(effectiveSubstatements); - Collection> substatementsToBuild = Collections2.filter(substatementsInit, - StmtContext::isSupportedToBuildEffective); - if (!buildUnknownSubstatements) { - this.unknownSubstatementsToBuild = ImmutableList.copyOf(Collections2.filter(substatementsToBuild, - IS_UNKNOWN_STATEMENT_CONTEXT)); - substatementsToBuild = Collections2.filter(substatementsToBuild, IS_NOT_UNKNOWN_STATEMENT_CONTEXT); - } else { - this.unknownSubstatementsToBuild = ImmutableList.of(); - } - - this.substatements = ImmutableList.copyOf(Collections2.transform(substatementsToBuild, StatementContextBase::buildEffective)); - } - - Collection> getOmittedUnknownSubstatements() { - return Collections2.transform(unknownSubstatementsToBuild, StatementContextBase::buildEffective); + this.substatements = ImmutableList.copyOf(Collections2.transform(Collections2.filter(substatementsInit, + StmtContext::isSupportedToBuildEffective), StatementContextBase::buildEffective)); } @Override @@ -100,11 +69,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 da4a2a92ec..fd9623b0bc 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/Bug4456Test.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java index 01cea68b48..3fa3f208cf 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/Bug4456Test.java @@ -22,10 +22,11 @@ import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode; import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; +import org.opendaylight.yangtools.yang.parser.spi.meta.SomeModifiersUnresolvedException; import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; public class Bug4456Test { - @Test + @Test(expected=SomeModifiersUnresolvedException.class) public void test() throws IOException, URISyntaxException, SourceException, ReactorException { SchemaContext schema = StmtTestUtils.parseYangSources("/bugs/bug4456"); assertNotNull(schema);