BUG-2459: reuse EXI Transmogrifier 11/13311/19
authorRobert Varga <rovarga@cisco.com>
Tue, 2 Dec 2014 14:57:27 +0000 (15:57 +0100)
committerRobert Varga <rovarga@cisco.com>
Thu, 18 Dec 2014 15:58:17 +0000 (16:58 +0100)
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>
opendaylight/netconf/netconf-client/src/test/java/org/opendaylight/controller/netconf/client/NetconfClientSessionTest.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSession.java
opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfMessageToEXIEncoder.java
opendaylight/netconf/netconf-netty-util/src/test/java/org/opendaylight/controller/netconf/nettyutil/handler/NetconfEXIHandlersTest.java

index e11be554d0df948700dc37b8650db0e3c5722172..103585cf7e330b1c3173c16a3feb23a6aa2b6ccc 100644 (file)
@@ -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();
index 8eb792b6cdfc368930db066dec991abdf47a3ea8..13b72bc62c460ffdec7da204e2c9297c4914f7b7 100644 (file)
@@ -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);
 
index e90bc7916dc03fd210e2b69048161d944fda10e4..aceb6ac520f5568120c8538fb320f72f2c8418e4 100644 (file)
@@ -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();
         }
     }
 }
index 4f804abfe8e4478fffb89b69e817afc1157665d0..1972cb181fe0cfac7483a59ab06112717676f531 100644 (file)
@@ -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));