BUG-2459: reuse EXI Transmogrifier

EXI Transmogrifier performs an internal reset whenever we acquire the
SAXTransmogrifier. This means that we are free to reuse the instance
between individual invocations as long as we can guarantee there are no
concurrent access. That is guaranteed by our Handler not being Shared.

Change-Id: Iba141915000b016579b273d4413ecd205f8da777
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/opendaylight/netconf/netconf-client/src/test/java/org/opendaylight/controller/netconf/client/NetconfClientSessionTest.java b/opendaylight/netconf/netconf-client/src/test/java/org/opendaylight/controller/netconf/client/NetconfClientSessionTest.java
index e11be55..103585c 100644 (file)
--- a/opendaylight/netconf/netconf-client/src/test/java/org/opendaylight/controller/netconf/client/NetconfClientSessionTest.java
+++ b/opendaylight/netconf/netconf-client/src/test/java/org/opendaylight/controller/netconf/client/NetconfClientSessionTest.java
@@ -54,7 +54,7 @@ public class NetconfClientSessionTest {
         Mockito.doReturn("").when(channelHandler).toString();
 
         NetconfClientSession session = new NetconfClientSession(sessionListener, channel, sessId, caps);
-        final NetconfMessageToEXIEncoder exiEncoder = new NetconfMessageToEXIEncoder(codec);
+        final NetconfMessageToEXIEncoder exiEncoder = NetconfMessageToEXIEncoder.create(codec);
         final NetconfEXIToMessageDecoder exiDecoder = new NetconfEXIToMessageDecoder(codec);
         session.addExiHandlers(exiDecoder, exiEncoder);
         session.stopExiCommunication();

diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java
index 8eb792b..13b72bc 100644 (file)
--- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java
+++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java
@@ -25,6 +25,7 @@ import org.opendaylight.controller.netconf.nettyutil.handler.exi.EXIParameters;
 import org.opendaylight.controller.netconf.util.xml.XmlElement;
 import org.opendaylight.protocol.framework.AbstractProtocolSession;
 import org.openexi.proc.common.EXIOptionsException;
+import org.openexi.sax.TransmogrifierException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -123,7 +124,14 @@ public abstract class AbstractNetconfSession<S extends NetconfSession, L extends
         }
 
         final NetconfEXICodec exiCodec = new NetconfEXICodec(exiParams.getOptions());
-        final NetconfMessageToEXIEncoder exiEncoder = new NetconfMessageToEXIEncoder(exiCodec);
+        final NetconfMessageToEXIEncoder exiEncoder;
+        try {
+            exiEncoder = NetconfMessageToEXIEncoder.create(exiCodec);
+        } catch (EXIOptionsException | TransmogrifierException e) {
+            LOG.warn("Failed to instantiate EXI encoder for {} on session {}", exiCodec, this, e);
+            throw new IllegalStateException("Cannot instantiate encoder for options", e);
+        }
+
         final NetconfEXIToMessageDecoder exiDecoder = new NetconfEXIToMessageDecoder(exiCodec);
         addExiHandlers(exiDecoder, exiEncoder);
 

diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java
index e90bc79..aceb6ac 100644 (file)
--- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java
+++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java
@@ -14,6 +14,7 @@ import io.netty.channel.ChannelHandlerContext;
 import io.netty.handler.codec.MessageToByteEncoder;
 import java.io.IOException;
 import java.io.OutputStream;
+import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
 import javax.xml.transform.dom.DOMSource;
 import javax.xml.transform.sax.SAXResult;
@@ -23,13 +24,23 @@ import org.openexi.sax.Transmogrifier;
 import org.openexi.sax.TransmogrifierException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.xml.sax.ContentHandler;
 
 public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder<NetconfMessage> {
     private static final Logger LOG = LoggerFactory.getLogger(NetconfMessageToEXIEncoder.class);
-    private final NetconfEXICodec codec;
+    /**
+     * This class is not marked as shared, so it can be attached to only a single channel,
+     * which means that {@link #encode(ChannelHandlerContext, NetconfMessage, ByteBuf)}
+     * cannot be invoked concurrently. Hence we can reuse the transmogrifier.
+     */
+    private final Transmogrifier transmogrifier;
 
-    public NetconfMessageToEXIEncoder(final NetconfEXICodec codec) {
-        this.codec = Preconditions.checkNotNull(codec);
+    private NetconfMessageToEXIEncoder(final Transmogrifier transmogrifier) {
+        this.transmogrifier = Preconditions.checkNotNull(transmogrifier);
+    }
+
+    public static NetconfMessageToEXIEncoder create(final NetconfEXICodec codec) throws EXIOptionsException, TransmogrifierException {
+        return new NetconfMessageToEXIEncoder(codec.getTransmogrifier());
     }
 
     @Override
@@ -37,10 +48,15 @@ public final class NetconfMessageToEXIEncoder extends MessageToByteEncoder<Netco
         LOG.trace("Sent to encode : {}", msg);
 
         try (final OutputStream os = new ByteBufOutputStream(out)) {
-            final Transmogrifier transmogrifier = codec.getTransmogrifier();
             transmogrifier.setOutputStream(os);
-
-            ThreadLocalTransformers.getDefaultTransformer().transform(new DOMSource(msg.getDocument()), new SAXResult(transmogrifier.getSAXTransmogrifier()));
+            final ContentHandler handler = transmogrifier.getSAXTransmogrifier();
+            final Transformer transformer = ThreadLocalTransformers.getDefaultTransformer();
+            transformer.transform(new DOMSource(msg.getDocument()), new SAXResult(handler));
+        } finally {
+            // Make sure we do not retain any reference to state by removing
+            // the output stream reference and resetting internal state.
+            transmogrifier.setOutputStream(null);
+            transmogrifier.getSAXTransmogrifier();
         }
     }
 }

diff --git a/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIHandlersTest.java b/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIHandlersTest.java
index 4f804ab..1972cb1 100644 (file)
--- a/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIHandlersTest.java
+++ b/opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIHandlersTest.java
@@ -10,7 +10,6 @@ package org.opendaylight.controller.netconf.nettyutil.handler;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
-
 import com.google.common.collect.Lists;
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
@@ -41,7 +40,7 @@ public class NetconfEXIHandlersTest {
     @Before
     public void setUp() throws Exception {
         final NetconfEXICodec codec = new NetconfEXICodec(new EXIOptions());
-        netconfMessageToEXIEncoder = new NetconfMessageToEXIEncoder(codec);
+        netconfMessageToEXIEncoder = NetconfMessageToEXIEncoder.create(codec);
         netconfEXIToMessageDecoder = new NetconfEXIToMessageDecoder(codec);
 
         msg = new NetconfMessage(XmlUtil.readXmlToDocument(msgAsString));