Qpid
  1. Qpid
  2. QPID-3553

C++ broker does not handle multiple ConnectionTuneOk cleanly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.13
    • Component/s: C++ Broker
    • Labels:
      None

      Description

      If the C++ broker is sent ConnectionTuneOk more than once with heartbeat set, it creates that many timer tasks for the heartbeat. This will result in a case where a heartbeat task is triggered for a connection that has already been deleted.

      Steps to reproduce:

      1) Start broker:

      MALLOC_MMAP_THRESHOLD_=8 qpidd

      The MALLOC_MMAP_THRESHOLD_ is to catch the use-after-free immediately rather than having to depend on a heap corruption

      2) Run the following python script:

      import struct, time
      from qpid.framing import OpEncoder, SegmentEncoder, FrameEncoder
      from qpid.util import connect
      from qpid.ops import *

      def encode(op):
      print "Sending:", op
      op_enc = OpEncoder()
      seg_enc = SegmentEncoder()
      frame_enc = FrameEncoder()

      op_enc.write(op)
      seg_enc.write(*op_enc.read())
      frame_enc.write(*seg_enc.read())
      bytes = frame_enc.read()
      print " bytes:", repr(bytes)
      return bytes

      conn = connect("127.0.0.1", 5672)
      conn.send(struct.pack("!4s4B", "AMQP", 1, 1, 0, 10))
      conn.send(encode(ConnectionTuneOk(heartbeat=1)))
      conn.send(encode(ConnectionTuneOk(heartbeat=1)))
      conn.send(encode(ConnectionOpen(virtual_host="vhost-blah",channel=0)))
      time.sleep(3)

      1. multipleConnectionTuneOk.patch
        0.9 kB
        Pavel Moravec
      2. qpid-duplicate-timer.patch
        0.6 kB
        Siddhesh Poyarekar

        Activity

        Hide
        Siddhesh Poyarekar added a comment -

        Do not add the heartbeat and timeout timers if the heartbeat timer already exists.

        Show
        Siddhesh Poyarekar added a comment - Do not add the heartbeat and timeout timers if the heartbeat timer already exists.
        Hide
        Pavel Moravec added a comment -

        An improvement to Siddhesh's patch to explicitly && independently set
        heartbeatTimer and timeoutTimer timers only if they are not set yet. Currently
        it does not improve anything as both timers are either both set or none set,
        but to prevent future issue if these timers could be set in other places
        independently on each other.

        Show
        Pavel Moravec added a comment - An improvement to Siddhesh's patch to explicitly && independently set heartbeatTimer and timeoutTimer timers only if they are not set yet. Currently it does not improve anything as both timers are either both set or none set, but to prevent future issue if these timers could be set in other places independently on each other.
        Hide
        Gordon Sim added a comment -

        Many thanks Siddhesh and Pavel!

        Show
        Gordon Sim added a comment - Many thanks Siddhesh and Pavel!

          People

          • Assignee:
            Gordon Sim
            Reporter:
            Siddhesh Poyarekar
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development