You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2014/03/29 00:31:26 UTC

[04/13] git commit: TS-2675: metalink: Fix crash if INKVConnInternal::do_io_close() gets called after a message is already complete

TS-2675: metalink: Fix crash if INKVConnInternal::do_io_close() gets called after a message is already complete


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/ccc549e9
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/ccc549e9
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/ccc549e9

Branch: refs/heads/master
Commit: ccc549e9bbc394e95cf7d1852f7d8a73efb1b4db
Parents: 54316fd
Author: Jack Bates <ja...@nottheoilrig.com>
Authored: Thu Mar 13 13:59:01 2014 -0700
Committer: James Peach <jp...@apache.org>
Committed: Fri Mar 28 16:23:54 2014 -0700

----------------------------------------------------------------------
 plugins/experimental/metalink/metalink.cc       | 36 ++++----
 .../metalink/test/pipeliningDisconnect          | 93 ++++++++++++++++++++
 2 files changed, 108 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc549e9/plugins/experimental/metalink/metalink.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/metalink/metalink.cc b/plugins/experimental/metalink/metalink.cc
index 5def165..8ebd2cc 100644
--- a/plugins/experimental/metalink/metalink.cc
+++ b/plugins/experimental/metalink/metalink.cc
@@ -261,22 +261,27 @@ vconn_write_ready(TSCont contp, void * /* edata ATS_UNUSED */)
 
   TransformData *transform_data = (TransformData *) TSContDataGet(contp);
 
+  int closed = TSVConnClosedGet(contp);
+  if (closed) {
+    TSContDestroy(contp);
+
+    /* Avoid failed assert "sdk_sanity_check_iocore_structure(bufp) ==
+     * TS_SUCCESS" in TSIOBufferDestroy() if the response is 304 Not Modified */
+    if (transform_data->output_bufp) {
+      TSIOBufferDestroy(transform_data->output_bufp);
+    }
+
+    TSfree(transform_data);
+
+    return 0;
+  }
+
   TSVIO input_viop = TSVConnWriteVIOGet(contp);
 
   /* Initialize data here because can't call TSVConnWrite() before
    * TS_HTTP_RESPONSE_TRANSFORM_HOOK */
   if (!transform_data->output_bufp) {
-
-    /* Avoid failed assert "sdk_sanity_check_iocore_structure(connp) ==
-     * TS_SUCCESS" in TSVConnWrite() if the response is 304 Not Modified */
     TSVConn output_connp = TSTransformOutputVConnGet(contp);
-    if (!output_connp) {
-      TSContDestroy(contp);
-
-      TSfree(transform_data);
-
-      return 0;
-    }
 
     transform_data->output_bufp = TSIOBufferCreate();
     TSIOBufferReader readerp = TSIOBufferReaderAlloc(transform_data->output_bufp);
@@ -322,17 +327,6 @@ vconn_write_ready(TSCont contp, void * /* edata ATS_UNUSED */)
 
   int ntodo = TSVIONTodoGet(input_viop);
   if (ntodo) {
-
-    /* Don't update the downstream nbytes because the input isn't complete */
-    if (!readerp) {
-      TSContDestroy(contp);
-
-      TSIOBufferDestroy(transform_data->output_bufp);
-      TSfree(transform_data);
-
-      return 0;
-    }
-
     TSVIOReenable(transform_data->output_viop);
 
     TSContCall(TSVIOContGet(input_viop), TS_EVENT_VCONN_WRITE_READY, input_viop);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/ccc549e9/plugins/experimental/metalink/test/pipeliningDisconnect
----------------------------------------------------------------------
diff --git a/plugins/experimental/metalink/test/pipeliningDisconnect b/plugins/experimental/metalink/test/pipeliningDisconnect
new file mode 100755
index 0000000..152e2cf
--- /dev/null
+++ b/plugins/experimental/metalink/test/pipeliningDisconnect
@@ -0,0 +1,93 @@
+#!/usr/bin/env python
+
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed
+# with this work for additional information regarding copyright
+# ownership.  The ASF licenses this file to you under the Apache
+# License, Version 2.0 (the "License"); you may not use this file
+# except in compliance with the License.  You may obtain a copy of the
+# License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.  See the License for the specific language governing
+# permissions and limitations under the License.
+
+print '''1..1 pipeliningDisconnect
+# The proxy doesn't crash if INKVConnInternal::do_io_close() gets called after
+# a message is already complete'''
+
+from twisted.internet import error, protocol, reactor, tcp
+from twisted.web import http
+
+def callback():
+  print 'ok 1 - Did the connection close before the proxy made the second request?'
+
+  reactor.stop()
+
+reactor.callLater(2, callback)
+
+class factory(http.HTTPFactory):
+  class protocol(http.HTTPChannel):
+    class requestFactory(http.Request):
+      def requestReceived(ctx, method, target, version):
+        class requestFactory(http.Request):
+          def requestReceived(ctx, method, target, version):
+
+            ctx.client = None
+            ctx.clientproto = version
+
+            ctx.write('pipeliningDisconnect')
+            ctx.finish()
+
+            # Open another connection
+            class factory(protocol.ClientFactory):
+              def clientConnectionFailed(ctx, connector, reason):
+                print 'not ok 1 - Did the proxy crash?  (Can\'t open another connection to it.)'
+
+                reactor.stop()
+
+              class protocol(protocol.Protocol):
+                def connectionMade(ctx):
+                  print 'ok 1 - The proxy didn\'t crash (opened another connection to it)'
+
+                  reactor.stop()
+
+            reactor.callLater(1, tcp.Connector('localhost', 8080, factory(), 30, None, reactor).connect)
+
+        ctx.channel.requestFactory = requestFactory
+
+        ctx.client = None
+        ctx.clientproto = version
+
+        ctx.write('pipeliningDisconnect')
+        ctx.finish()
+
+origin = tcp.Port(0, factory())
+origin.startListening()
+
+print '# Listening on {0}:{1}'.format(*origin.socket.getsockname())
+
+class factory(protocol.ClientFactory):
+  def clientConnectionFailed(ctx, connector, reason):
+
+    print 'Bail out!'
+    reason.printTraceback()
+
+    reactor.stop()
+
+  class protocol(protocol.Protocol):
+    def connectionMade(ctx):
+
+      # Somehow these magic words frequently cause
+      # INKVConnInternal::do_io_close() to get called after a message is
+      # already complete
+      ctx.transport.write('GET {0}:{1}/pipeliningDisconnect0 HTTP/1.1\r\n\r\nGET {0}:{1}/pipeliningDisconnect1 HTTP/1.1\r\n\r\n'.format(*origin.socket.getsockname()))
+      ctx.transport.loseConnection()
+
+tcp.Connector('localhost', 8080, factory(), 30, None, reactor).connect()
+
+reactor.run()