[
https://issues.apache.org/jira/browse/DIRMINA-1110?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17143013#comment-17143013
]
Jonathan Valliere commented on DIRMINA-1110:
--------------------------------------------
For the record, the above patch was reverted at
24fc810141081119273a71f61b08c94aeaf43d5c by my request because I'm not sure the
patch actually fixed anything.
> Ordered Executors concurrency
> -----------------------------
>
> Key: DIRMINA-1110
> URL: https://issues.apache.org/jira/browse/DIRMINA-1110
> Project: MINA
> Issue Type: Improvement
> Affects Versions: 2.1.2
> Reporter: Guus der Kinderen
> Priority: Major
> Attachments: mina-ordered-executors.patch
>
>
> MINA contains two ThreadPoolExecutor implementations that maintains the order
> of IoEvents per session:
> * OrderedThreadPoolExecutor
> * PriorityThreadPoolExecutor
> This issue applies to both.
> A private class called {{SessionTasksQueue}} (confusingly using different
> names in both implementations, which is an undesired side-effect having code
> duplication) is used to represent the ordered collection of events to be
> processed for each individual session. It also contains a boolean that
> represents the state of the queue prior to addition of the task: 'true' if
> the collection was empty ("processing complete"), otherwise 'false'. This
> queue is stored as an attribute on the session.
> An {{IoEvent}} is _scheduled_ for execution by being passed to the
> {{execute}} method. "Scheduling" an {{IoEvent}} involves identifying the
> session that it belongs to, and placing it in its SessionTaskQueue. Finally,
> the session itself is, conditionally (more in this later), placed in a queue
> named {{waitingSessions}}.
> The {{IoEvent}} execution itself is implemented by {{Runnable}}
> implementations called {{Worker}}. These workers take their workload from the
> {{waitingSessions}} queue, doing blocking polls.
> The placing of the Session on the {{waitingSessions}} queue depends on the
> state of the {{SessionTasksQueue}}. If it was empty ("processingComplete"),
> the session is placed on the {{waitingSessions}} queue. There is comment that
> describes this as follows:
> {quote}As the tasksQueue was empty, the task has been executed immediately,
> so we can move the session to the queue of sessions waiting for
> completion.{quote}
> As an aside: I believe this comment to be misleading: there's no guarantee
> that the task has actually been executed immediately, as workers might still
> be working on other threads. On top of that, as both the production on, and
> consumption of that queue is guarded by the same mutex, I think it's quite
> impossible that the task has already been executed. A more proper comment
> would be:
> {quote}Processing of the tasks queue of this session is currently not
> scheduled or underway. As new tasks have now been added, the session needs to
> be offered for processing.{quote}
> The determination if the session needs to be offered up for processing is
> based on an evaluation of the state of the {{sessionTasksQueue}} that happens
> under a mutex. The actual offer of the session for processing (adding the
> session to the {{waitingSessions}} queue, is not. We believe, but have not
> been able to conclusively prove, that this can lead to concurrency issues,
> where a session might not be queued, even though it has tasks to be executed.
> Nonetheless, we suggest moving the addition of the session to
> {{waitingSessions}} into the guarded code block. At the very least, this
> reduces code complexity.
> The patch attached to this issue applies this change. It also changes the
> name of the Session tasks queue in {{PriorityThreadPoolExecutor}}, to match
> the name used in {{OrderedThreadPoolExecutor}}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
