Refactor ExtensionStatementSupport 18/90918/3
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 3 Jul 2020 14:52:15 +0000 (16:52 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Sat, 4 Jul 2020 11:13:51 +0000 (13:13 +0200)
Using RecursiveObjectLeaker requires integration with object
construction and is inherently dangerous.

As it turns out, we can solve this problem differently, without
having to rely on this magic by pre-allocating the resulting
effective statement and populating it into a thread-local map.

That allows us to pick up that object for purposes of including
it in substatements -- thus breaking the recursion. Once we have
acquired substatements, the real build methods just fill them
into the pre-allocated object are return it.

On exit we check whether we have cleared the state map and clean
it up automatically, as this is not expected to be a major
performance problem.

JIRA: YANGTOOLS-1122
Change-Id: Iebbbffd6f62fa57ce496a2ab7bc8e5792198d3a5
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionEffectiveStatement.java
yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/stmt/ExtensionStatement.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/BaseQNameStatementSupport.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/EmptyExtensionStatement.java [moved from yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementImpl.java with 58% similarity]
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionEffectiveStatementImpl.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/ExtensionStatementSupport.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java [new file with mode: 0644]

index d8a8586c9a171608fa689dfa56564415c9bbe2f2..346c332d491e88ec8784456b89038a523a94851d 100644 (file)
@@ -8,8 +8,13 @@
 package org.opendaylight.yangtools.yang.model.api.stmt;
 
 import com.google.common.annotations.Beta;
+import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
+import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition;
 
 @Beta
 public interface ExtensionEffectiveStatement extends NamespacedEffectiveStatement<ExtensionStatement> {
-
+    @Override
+    default StatementDefinition statementDefinition() {
+        return YangStmtMapping.EXTENSION;
+    }
 }
index 01f712047d4dac462f0d5997b125bff45f17e4f3..b97835cbede43c17939445859b814c6a8f1252a0 100644 (file)
@@ -10,8 +10,15 @@ package org.opendaylight.yangtools.yang.model.api.stmt;
 import java.util.Optional;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
+import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition;
 
 public interface ExtensionStatement extends DocumentedDeclaredStatement.WithStatus<QName> {
+    @Override
+    default StatementDefinition statementDefinition() {
+        return YangStmtMapping.EXTENSION;
+    }
+
     default @Nullable ArgumentStatement getArgument() {
         final Optional<ArgumentStatement> opt = findFirstDeclaredSubstatement(ArgumentStatement.class);
         return opt.isPresent() ? opt.get() : null;
index bbc2d6c94acd749f310428949e8b4bcb51c1e9fc..5af1ba27831c23808cd6c5cd44f9bb48be8094ba 100644 (file)
@@ -48,7 +48,7 @@ public abstract class BaseQNameStatementSupport<D extends DeclaredStatement<QNam
     protected abstract @NonNull D createEmptyDeclared(@NonNull StmtContext<QName, D, ?> ctx);
 
     @Override
-    public final E createEffective(final StmtContext<QName, D, E> ctx) {
+    public E createEffective(final StmtContext<QName, D, E> ctx) {
         final D declared = ctx.buildDeclared();
         final ImmutableList<? extends EffectiveStatement<?, ?>> substatements =
                 BaseStatementSupport.buildEffectiveSubstatements(ctx);
@@ -9,11 +9,10 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension;
 
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement;
-import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractDeclaredStatement;
-import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredStatement.WithQNameArgument;
 
-final class ExtensionStatementImpl extends AbstractDeclaredStatement<QName> implements ExtensionStatement {
-    ExtensionStatementImpl(final StmtContext<QName, ExtensionStatement,?> context) {
-        super(context);
+final class EmptyExtensionStatement extends WithQNameArgument implements ExtensionStatement {
+    EmptyExtensionStatement(final QName argument) {
+        super(argument);
     }
 }
index e3de5c550b926755f60ae41a3edde64144abe523..13ef829a59ecb5ba3ce1ffd362d5c02c9de4582d 100644 (file)
@@ -7,26 +7,29 @@
  */
 package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension;
 
+import static com.google.common.base.Verify.verify;
+import static com.google.common.base.Verify.verifyNotNull;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.collect.ImmutableList;
 import java.util.ArrayDeque;
-import java.util.Collection;
 import java.util.Deque;
-import java.util.Optional;
 import org.eclipse.jdt.annotation.NonNull;
-import org.eclipse.jdt.annotation.Nullable;
-import org.opendaylight.yangtools.util.RecursiveObjectLeaker;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.ExtensionDefinition;
 import org.opendaylight.yangtools.yang.model.api.SchemaPath;
+import org.opendaylight.yangtools.yang.model.api.Status;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ArgumentEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.StatusEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.YinElementEffectiveStatement;
-import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractEffectiveDocumentedNodeWithStatus;
-import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredEffectiveStatement.DefaultArgument;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.EffectiveStatementMixins.DocumentedNodeMixin;
 
-final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedNodeWithStatus<QName, ExtensionStatement>
-        implements ExtensionDefinition, ExtensionEffectiveStatement {
+final class ExtensionEffectiveStatementImpl extends DefaultArgument<QName, ExtensionStatement>
+        implements ExtensionDefinition, ExtensionEffectiveStatement, DocumentedNodeMixin<QName, ExtensionStatement> {
     private static final class RecursionDetector extends ThreadLocal<Deque<ExtensionEffectiveStatementImpl>> {
         boolean check(final ExtensionEffectiveStatementImpl current) {
             final Deque<ExtensionEffectiveStatementImpl> stack = get();
@@ -61,85 +64,50 @@ final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedN
 
     private static final RecursionDetector TOSTRING_DETECTOR = new RecursionDetector();
 
-    private final @NonNull QName qname;
-    private final @Nullable String argument;
-    private final @NonNull SchemaPath schemaPath;
-
-    private final boolean yin;
-
-    private ExtensionEffectiveStatementImpl(
-            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx) {
-        super(ctx);
-        this.qname = ctx.coerceStatementArgument();
-        this.schemaPath = ctx.getSchemaPath().get();
-
-        // initFields
-        final Optional<ArgumentEffectiveStatement> optArgumentSubstatement = findFirstEffectiveSubstatement(
-            ArgumentEffectiveStatement.class);
-        if (optArgumentSubstatement.isPresent()) {
-            final ArgumentEffectiveStatement argumentStatement = optArgumentSubstatement.get();
-            this.argument = argumentStatement.argument().getLocalName();
-            this.yin = argumentStatement.findFirstEffectiveSubstatement(YinElementEffectiveStatement.class)
-                    .map(YinElementEffectiveStatement::argument).orElse(Boolean.FALSE).booleanValue();
-        } else {
-            this.argument = null;
-            this.yin = false;
-        }
-    }
+    private final @NonNull SchemaPath path;
 
-    /**
-     * Create a new ExtensionEffectiveStatement, dealing with potential recursion.
-     *
-     * @param ctx Statement context
-     * @return A potentially under-initialized instance
-     */
-    static ExtensionEffectiveStatement create(
-            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx) {
-        // Look at the thread-local leak in case we are invoked recursively
-        final ExtensionEffectiveStatementImpl existing = RecursiveObjectLeaker.lookup(ctx,
-            ExtensionEffectiveStatementImpl.class);
-        if (existing != null) {
-            // Careful! this object is not fully initialized!
-            return existing;
-        }
+    private volatile Object substatements;
 
-        RecursiveObjectLeaker.beforeConstructor(ctx);
-        try {
-            // This result is fine, we know it has been completely initialized
-            return new ExtensionEffectiveStatementImpl(ctx);
-        } finally {
-            RecursiveObjectLeaker.afterConstructor(ctx);
-        }
-    }
-
-    @Override
-    protected Collection<? extends EffectiveStatement<?, ?>> initSubstatements(
-            final Collection<? extends StmtContext<?, ?, ?>> substatementsInit) {
-        // WARNING: this leaks an incompletely-initialized object
-        RecursiveObjectLeaker.inConstructor(this);
-
-        return super.initSubstatements(substatementsInit);
+    ExtensionEffectiveStatementImpl(final ExtensionStatement declared, final SchemaPath path) {
+        super(declared);
+        this.path = requireNonNull(path);
     }
 
     @Override
     public QName getQName() {
-        return qname;
+        return argument();
     }
 
     @Override
     @Deprecated
     public SchemaPath getPath() {
-        return schemaPath;
+        return path;
     }
 
     @Override
     public String getArgument() {
-        return argument;
+        return findFirstEffectiveSubstatementArgument(ArgumentEffectiveStatement.class)
+                .map(QName::getLocalName)
+                .orElse(null);
     }
 
     @Override
     public boolean isYinElement() {
-        return yin;
+        return findFirstEffectiveSubstatement(ArgumentEffectiveStatement.class)
+                .flatMap(arg -> arg.findFirstEffectiveSubstatementArgument(YinElementEffectiveStatement.class))
+                .orElse(Boolean.FALSE)
+                .booleanValue();
+    }
+
+    @Override
+    public ImmutableList<? extends EffectiveStatement<?, ?>> effectiveSubstatements() {
+        final Object local = verifyNotNull(substatements, "Substatements are not yet initialized");
+        return unmaskList(local);
+    }
+
+    @Override
+    public Status getStatus() {
+        return findFirstEffectiveSubstatementArgument(StatusEffectiveStatement.class).orElse(Status.CURRENT);
     }
 
     @Override
@@ -151,23 +119,28 @@ final class ExtensionEffectiveStatementImpl extends AbstractEffectiveDocumentedN
         TOSTRING_DETECTOR.push(this);
         try {
             return ExtensionEffectiveStatementImpl.class.getSimpleName() + "["
-                    + "argument=" + argument
-                    + ", qname=" + qname
-                    + ", schemaPath=" + schemaPath
+                    + "argument=" + getArgument()
+                    + ", qname=" + getQName()
+                    + ", schemaPath=" + path
+                    + ", yin=" + isYinElement()
                     + ", extensionSchemaNodes=" + getUnknownSchemaNodes()
-                    + ", yin=" + yin
                     + "]";
         } finally {
             TOSTRING_DETECTOR.pop();
         }
     }
 
+    void setSubstatements(final ImmutableList<? extends EffectiveStatement<?, ?>> newSubstatements) {
+        verify(substatements == null, "Substatements already initialized");
+        substatements = maskList(newSubstatements);
+    }
+
     private String recursedToString() {
         return ExtensionEffectiveStatementImpl.class.getSimpleName() + "["
-                + "argument=" + argument
-                + ", qname=" + qname
-                + ", schemaPath=" + schemaPath
-                + ", yin=" + yin
+                + "argument=" + getArgument()
+                + ", qname=" + getQName()
+                + ", schemaPath=" + path
+                + ", yin=" + isYinElement()
                 + " <RECURSIVE> ]";
     }
 }
index 144b7f1cdd87973dc236de76e0351517a9ba1311..1752e7098296fd4f1e8c2f3717b0762bb108f74d 100644 (file)
@@ -7,15 +7,24 @@
  */
 package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension;
 
+import static com.google.common.base.Verify.verify;
+import static com.google.common.base.Verify.verifyNotNull;
+
+import com.google.common.collect.ImmutableList;
+import java.util.IdentityHashMap;
+import java.util.Map;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.openconfig.model.api.OpenConfigStatements;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
+import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
+import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ArgumentStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.YinElementStatement;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.BaseQNameStatementSupport;
 import org.opendaylight.yangtools.yang.parser.spi.ExtensionNamespace;
-import org.opendaylight.yangtools.yang.parser.spi.meta.AbstractQNameStatementSupport;
 import org.opendaylight.yangtools.yang.parser.spi.meta.StatementDefinitionNamespace;
 import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext;
 import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable;
@@ -23,7 +32,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils;
 import org.opendaylight.yangtools.yang.parser.spi.meta.SubstatementValidator;
 
 public final class ExtensionStatementSupport
-        extends AbstractQNameStatementSupport<ExtensionStatement, ExtensionEffectiveStatement> {
+        extends BaseQNameStatementSupport<ExtensionStatement, ExtensionEffectiveStatement> {
     private static final SubstatementValidator SUBSTATEMENT_VALIDATOR = SubstatementValidator.builder(YangStmtMapping
         .EXTENSION)
         .addOptional(YangStmtMapping.ARGUMENT)
@@ -32,6 +41,8 @@ public final class ExtensionStatementSupport
         .addOptional(YangStmtMapping.STATUS)
         .build();
     private static final ExtensionStatementSupport INSTANCE = new ExtensionStatementSupport();
+    private static final ThreadLocal<Map<StmtContext<?, ?, ?>, ExtensionEffectiveStatementImpl>> TL_BUILDERS =
+            new ThreadLocal<>();
 
     private ExtensionStatementSupport() {
         super(YangStmtMapping.EXTENSION);
@@ -46,17 +57,6 @@ public final class ExtensionStatementSupport
         return StmtContextUtils.parseIdentifier(ctx, value);
     }
 
-    @Override
-    public ExtensionStatement createDeclared(final StmtContext<QName, ExtensionStatement,?> ctx) {
-        return new ExtensionStatementImpl(ctx);
-    }
-
-    @Override
-    public ExtensionEffectiveStatement createEffective(
-            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx) {
-        return ExtensionEffectiveStatementImpl.create(ctx);
-    }
-
     @Override
     public void onStatementDefinitionDeclared(
             final Mutable<QName, ExtensionStatement, ExtensionEffectiveStatement> stmt) {
@@ -84,4 +84,70 @@ public final class ExtensionStatementSupport
     protected SubstatementValidator getSubstatementValidator() {
         return SUBSTATEMENT_VALIDATOR;
     }
+
+    @Override
+    protected ExtensionStatement createDeclared(final StmtContext<QName, ExtensionStatement, ?> ctx,
+            final ImmutableList<? extends DeclaredStatement<?>> substatements) {
+        return new RegularExtensionStatement(ctx.coerceStatementArgument(), substatements);
+    }
+
+    @Override
+    protected ExtensionStatement createEmptyDeclared(final StmtContext<QName, ExtensionStatement, ?> ctx) {
+        return new EmptyExtensionStatement(ctx.coerceStatementArgument());
+    }
+
+    @Override
+    public ExtensionEffectiveStatement createEffective(
+            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx) {
+        Map<StmtContext<?, ?, ?>, ExtensionEffectiveStatementImpl> tl = TL_BUILDERS.get();
+        if (tl == null) {
+            tl = new IdentityHashMap<>();
+            TL_BUILDERS.set(tl);
+        }
+
+        final ExtensionEffectiveStatementImpl existing = tl.get(ctx);
+        if (existing != null) {
+            // Implies non-empty map, no cleanup necessary
+            return existing;
+        }
+
+        try {
+            final ExtensionEffectiveStatementImpl created = new ExtensionEffectiveStatementImpl(ctx.buildDeclared(),
+                ctx.getSchemaPath().get());
+            verify(tl.put(ctx, created) == null);
+            try {
+                return super.createEffective(ctx);
+            } finally {
+                verify(tl.remove(ctx) == created);
+
+            }
+        } finally {
+            if (tl.isEmpty()) {
+                TL_BUILDERS.remove();
+            }
+        }
+    }
+
+    @Override
+    protected ExtensionEffectiveStatement createEffective(
+            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx,
+            final ExtensionStatement declared,
+            final ImmutableList<? extends EffectiveStatement<?, ?>> substatements) {
+        return finishCreate(ctx, substatements);
+    }
+
+    @Override
+    protected ExtensionEffectiveStatement createEmptyEffective(
+            final StmtContext<QName, ExtensionStatement, ExtensionEffectiveStatement> ctx,
+            final ExtensionStatement declared) {
+        return finishCreate(ctx, ImmutableList.of());
+    }
+
+    private static @NonNull ExtensionEffectiveStatement finishCreate(final StmtContext<?, ?, ?> ctx,
+            final ImmutableList<? extends EffectiveStatement<?, ?>> substatements) {
+        final ExtensionEffectiveStatementImpl ret = verifyNotNull(verifyNotNull(TL_BUILDERS.get(),
+            "Statement build state not initialized").get(ctx), "No build state found for %s", ctx);
+        ret.setSubstatements(substatements);
+        return ret;
+    }
 }
\ No newline at end of file
diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/extension/RegularExtensionStatement.java
new file mode 100644 (file)
index 0000000..7baacd9
--- /dev/null
@@ -0,0 +1,20 @@
+/*
+ * Copyright (c) 2020 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.extension;
+
+import com.google.common.collect.ImmutableList;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
+import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionStatement;
+import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractDeclaredStatement.WithQNameArgument.WithSubstatements;
+
+final class RegularExtensionStatement extends WithSubstatements implements ExtensionStatement {
+    RegularExtensionStatement(final QName argument, final ImmutableList<? extends DeclaredStatement<?>> substatements) {
+        super(argument, substatements);
+    }
+}