[jira] Created: (QPID-1515) [Java Client] Issues with dispatcher threading

["Marnie McCormack (JIRA)" <qp...@incubator.apache.org>]

[Java Client] Issues with dispatcher threading
----------------------------------------------

                 Key: QPID-1515
                 URL: https://issues.apache.org/jira/browse/QPID-1515
             Project: Qpid
          Issue Type: Bug
    Affects Versions: M3
            Reporter: Marnie McCormack
            Assignee: Rafael H. Schloming


I believe there are several issues in the dispatcher code that need to be rectified, so if we close this JIRA (was comment in QPID-823) we should open at least one new one to cover these issues: 

  - The dispatcher thread uses it's own _closed variable which shadows the session's. It's unclear why this is so, and at least some of the leaked dispatcher threads observed would have exited had the dispatcher code reference the session's closed variable rather than its own. 

  - The fact that the dispatcher thread is instantiated on demand seems both unnecessary and accident prone. This is also responsible for some of the leaked threads observed, particularly in combination with the above issue where a dispatcher thread is created on demand after the session is closed. 

  - The dispatcher object itself has code on it that would be better as private methods on the session. Because this code is on the dispatcher class, the dispatcher has to be re-instantiated just to call it. This was the cause of the leaked dispatcher threads as Arnaud describes above. 

  - The code in confirmConsumerCancelled(...) that checks if the dispatcher is null and the starts it if necessary is not thread safe. This should either be synchronized, or eliminated entirely by not instantiating the dispatcher thread on demand and/or moving rejectPending(...) out to the session so it can be called without access to the dispatcher object. 

  - As mentioned above, interrupt() is not a safe way to shutdown the dispatcher thread. As coded it could at least hypothetically interrupt the application while it's in the middle of a message handler, and, by causing the thread to exit after removing a message from the queue, but prior to handling or releasing the message, it may cause the client to drop messages should have either been handled or released. 



-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

[jira] Updated: (QPID-1515) [Java Client] Issues with dispatcher threading

["Marnie McCormack (JIRA)" <qp...@incubator.apache.org>]

     [ https://issues.apache.org/jira/browse/QPID-1515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Marnie McCormack updated QPID-1515:
-----------------------------------

    Component/s: Java Client

Adding component

> [Java Client] Issues with dispatcher threading
> ----------------------------------------------
>
>                 Key: QPID-1515
>                 URL: https://issues.apache.org/jira/browse/QPID-1515
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Client
>    Affects Versions: M3
>            Reporter: Marnie McCormack
>            Assignee: Rafael H. Schloming
>
> I believe there are several issues in the dispatcher code that need to be rectified, so if we close this JIRA (was comment in QPID-823) we should open at least one new one to cover these issues: 
>   - The dispatcher thread uses it's own _closed variable which shadows the session's. It's unclear why this is so, and at least some of the leaked dispatcher threads observed would have exited had the dispatcher code reference the session's closed variable rather than its own. 
>   - The fact that the dispatcher thread is instantiated on demand seems both unnecessary and accident prone. This is also responsible for some of the leaked threads observed, particularly in combination with the above issue where a dispatcher thread is created on demand after the session is closed. 
>   - The dispatcher object itself has code on it that would be better as private methods on the session. Because this code is on the dispatcher class, the dispatcher has to be re-instantiated just to call it. This was the cause of the leaked dispatcher threads as Arnaud describes above. 
>   - The code in confirmConsumerCancelled(...) that checks if the dispatcher is null and the starts it if necessary is not thread safe. This should either be synchronized, or eliminated entirely by not instantiating the dispatcher thread on demand and/or moving rejectPending(...) out to the session so it can be called without access to the dispatcher object. 
>   - As mentioned above, interrupt() is not a safe way to shutdown the dispatcher thread. As coded it could at least hypothetically interrupt the application while it's in the middle of a message handler, and, by causing the thread to exit after removing a message from the queue, but prior to handling or releasing the message, it may cause the client to drop messages should have either been handled or released. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.