Uploaded image for project: 'Qpid'
  1. Qpid
  2. QPID-3905

NullPointerException is thrown on rejecting messages whilst closing the connection

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.14, 0.15
    • Fix Version/s: 0.16
    • Component/s: Java Client
    • Labels:
      None

      Description

      NullPointerException is thrown on rejecting messages whilst closing the connection.

      The dispatcher is set to null at this point and accessing it causing the NullPointerException.

      2012-03-21 06:04:29,946 ERROR [AMQProtocolHandler] Exception processing frame
      java.lang.NullPointerException
      at org.apache.qpid.client.AMQSession$Dispatcher.rejectPending(AMQSession.java:3217)
      at org.apache.qpid.client.AMQSession.confirmConsumerCancelled(AMQSession.java:903)
      at org.apache.qpid.client.protocol.AMQProtocolSession.confirmConsumerCancelled(AMQProtocolSession.java:397)
      at org.apache.qpid.client.handler.BasicCancelOkMethodHandler.methodReceived(BasicCancelOkMethodHandler.java:53)
      at org.apache.qpid.client.handler.ClientMethodDispatcherImpl.dispatchBasicCancelOk(ClientMethodDispatcherImpl.java:126)
      at org.apache.qpid.framing.amqp_0_9.BasicCancelOkBodyImpl.execute(BasicCancelOkBodyImpl.java:110)
      at org.apache.qpid.client.state.AMQStateManager.methodReceived(AMQStateManager.java:114)
      at org.apache.qpid.client.protocol.AMQProtocolHandler.methodBodyReceived(AMQProtocolHandler.java:479)
      at org.apache.qpid.client.protocol.AMQProtocolSession.methodFrameReceived(AMQProtocolSession.java:455)
      at org.apache.qpid.framing.AMQMethodBodyImpl.handle(AMQMethodBodyImpl.java:97)
      at org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:436)
      at org.apache.qpid.client.protocol.AMQProtocolHandler.received(AMQProtocolHandler.java:121)
      at org.apache.qpid.transport.network.io.IoReceiver.run(IoReceiver.java:152)
      at java.lang.Thread.run(Thread.java:662)

        Activity

        Hide
        k-wall Keith Wall added a comment -

        I wonder if my change for QPID-3807 has exposed this defect.

        The method Dispatcher#rejectPending (oddly) refers to itself via the field AMQSession#_dispatcher rather than making method calls on itself. Before QPID-3807, updates to the fields were made in a thread-unsafe manner so it is possible that the Dispatcher, reading stale copies of the fields, allowed this bug to go unnoticed.

        I see no reason for #rejectPending to use _dispatcher, and think we should change it to call the method directly.

        We should probably consider having this included in 0-16.

        Show
        k-wall Keith Wall added a comment - I wonder if my change for QPID-3807 has exposed this defect. The method Dispatcher#rejectPending (oddly) refers to itself via the field AMQSession#_dispatcher rather than making method calls on itself. Before QPID-3807 , updates to the fields were made in a thread-unsafe manner so it is possible that the Dispatcher, reading stale copies of the fields, allowed this bug to go unnoticed. I see no reason for #rejectPending to use _dispatcher, and think we should change it to call the method directly. We should probably consider having this included in 0-16.
        Hide
        k-wall Keith Wall added a comment -

        Patch applied. Alex, could you review please?

        Show
        k-wall Keith Wall added a comment - Patch applied. Alex, could you review please?
        Hide
        gemmellr Robbie Gemmell added a comment -

        Changes look good to me. This ought to be requested for 0.16 since even if it could have potentially happened previously we have effectively made it far far more likely to show up since 0.14.

        Show
        gemmellr Robbie Gemmell added a comment - Changes look good to me. This ought to be requested for 0.16 since even if it could have potentially happened previously we have effectively made it far far more likely to show up since 0.14.
        Hide
        justi9 Justin Ross added a comment -

        Reviewed by Robbie. Approved for 0.16.

        Show
        justi9 Justin Ross added a comment - Reviewed by Robbie. Approved for 0.16.
        Hide
        gemmellr Robbie Gemmell added a comment -

        Changes merged to the 0.16 branch.

        Show
        gemmellr Robbie Gemmell added a comment - Changes merged to the 0.16 branch.

          People

          • Assignee:
            gemmellr Robbie Gemmell
            Reporter:
            alex.rufous Alex Rudyy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development