Qpid
  1. Qpid
  2. QPID-3777

Messaging API link binding is not unbound if client disconnects uncleanly

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 0.12
    • Fix Version/s: None
    • Component/s: C++ Broker, C++ Client
    • Labels:
      None

      Description

      Description of problem:
      Link bindings within the Messaging API (e.g. 'test.q;{create:always, node:

      {type:queue}

      ,link:{x-bindings:[

      {exchange:test.ex,queue:test.q,key:#}

      ]}}') are implemented solely within the client. Therefore, if the connection between the broker and the client is severed, the client will not be able to remove the link binding as it normally would during a clean client disconnect.

      How reproducible:
      100%

      Steps to Reproduce:
      1. Create a link binding via a C++ Messaging API sender or receiver
      2. Kill the client or sever the link between the client and broker

      Actual results:
      The link binding is still present in the broker

      Expected results:
      The link binding would be unbound when the broker discovers the client was disconnected

      1. QPID-3777.patch
        8 kB
        Jason Dillaman

        Activity

        Hide
        Jason Dillaman added a comment -

        I can see your point with regard to the client library changes. It would be trivial in our use case to have the client application append a new argument to indicate that the binding should be scoped w/ the session.

        In regards to the broker side tuple which stores the user id and url, my original implementation did not attempt to store the data. However, I was forced to store the data after discovering that 'SessionState::getConnection' would throw an assertion failure while ExchangeHandlerImpl's destructor was attempting to cleanup the bindings.

        Show
        Jason Dillaman added a comment - I can see your point with regard to the client library changes. It would be trivial in our use case to have the client application append a new argument to indicate that the binding should be scoped w/ the session. In regards to the broker side tuple which stores the user id and url, my original implementation did not attempt to store the data. However, I was forced to store the data after discovering that 'SessionState::getConnection' would throw an assertion failure while ExchangeHandlerImpl's destructor was attempting to cleanup the bindings.
        Hide
        Gordon Sim added a comment - - edited

        I don't like having the client library automatically mark link bindings as session scoped. That seems to me like something that should be explicitly asked for. For one thing it would invalidate a more 'normal' usage in durable/reliable subscriptions to an xml- or headers- exchange.

        The broker side change is a new feature/extension as much as a bug fix. I'm not un-sympathetic to the desire for such an extension. My concern would be to avoid having it tie us (and those using it, unless they explicitly so choose) to the pre-1.0 AMQP model. Put another way, while I'm not opposed to the broker side change, I would not myself encourage anyone to use it and would not want to have to keep supporting it, certainly not on 1.0 based sessions.

        One minor criticism of the broker side change is the inclusion of the user- and connection- identifiers in the stored tuple. I would argue those should be taken from the context of the unbind call when it is made. Though in practice there will be no difference, it seems more in line with the spirit of the code (where those are used for permission checking and therefore it is the permission of the unbind not the bind that counts).

        Show
        Gordon Sim added a comment - - edited I don't like having the client library automatically mark link bindings as session scoped. That seems to me like something that should be explicitly asked for. For one thing it would invalidate a more 'normal' usage in durable/reliable subscriptions to an xml- or headers- exchange. The broker side change is a new feature/extension as much as a bug fix. I'm not un-sympathetic to the desire for such an extension. My concern would be to avoid having it tie us (and those using it, unless they explicitly so choose) to the pre-1.0 AMQP model. Put another way, while I'm not opposed to the broker side change, I would not myself encourage anyone to use it and would not want to have to keep supporting it, certainly not on 1.0 based sessions. One minor criticism of the broker side change is the inclusion of the user- and connection- identifiers in the stored tuple. I would argue those should be taken from the context of the unbind call when it is made. Though in practice there will be no difference, it seems more in line with the spirit of the code (where those are used for permission checking and therefore it is the permission of the unbind not the bind that counts).
        Jason Dillaman made changes -
        Field Original Value New Value
        Attachment QPID-3777.patch [ 12511519 ]
        Hide
        Jason Dillaman added a comment -

        Proposed patch to clean-up link bindings

        Show
        Jason Dillaman added a comment - Proposed patch to clean-up link bindings
        Jason Dillaman created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Jason Dillaman
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development