[
https://issues.apache.org/jira/browse/DIRMINA-1156?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17474686#comment-17474686
]
Guus der Kinderen edited comment on DIRMINA-1156 at 1/12/22, 5:24 PM:
----------------------------------------------------------------------
Since your last comment in this issue, we've followed up in the mailing list.
To summarize:
{quote}I've asked a user that suffers from the problem to run with a slightly
modified version of MINA. The modification logs (and rethrows) any Throwable in
Worker's run(). This caused an Error to be logged! The Error was thrown by an
IoHandler implementation. Full details are available at
[https://igniterealtime.atlassian.net/browse/OF-2367?focusedCommentId=22738]
{quote}
Note that this problem affects _all_ ThreadPoolExecutors, not just the ordered
one!
I have written various unit tests to illustrate the issue, which are attached
to this issue (note that these are all new classes, except for
{{{}PriorityThreadPoolExecutorTest.java{}}}, which expands on a pre-existing
implementation).
The tests in [^PriorityThreadPoolExecutorTest.java]
[^OrderedThreadPoolExecutorTest.java] and
[^UnorderedThreadPoolExecutorTest.java] assert that the {{run()}} method of the
{{Worker}} class in each {{ThreadPoolExecutor}} implementation is susceptible
to this problem, but uses an implementation that bypasses some of the usage
patterns that would normally prevent this issue. These tests prove that the
implementation is weak, without immediately identifying an exploit of that
weakness.
The tests in [^DIRMINA1156Test.java] simulate one of the real-world causes of
the issue: having a {{Session}} that has a {{Handler}} that throws an {{Error}}
(I've included tests that throw checked and unchecked \{{Exception}}s, but
those do not trigger the bug).
Removing the 'finally' block that is inside the {{for (;;)}} block of Worker,
and making execution of {{idleWorkers.incrementAndGet();}} happen only when no
{{Throwable}} is thrown, will make all these tests go green. In other words,
replace:
{code:java}
try {
if (session != null) {
runTasks(getSessionTasksQueue(session)); }
} finally {
idleWorkers.incrementAndGet();
}
{code}
with this:
{code:java}
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
idleWorkers.incrementAndGet();
{code}
(a slightly different implementation is needed for PriorityThreadPoolExecutor).
was (Author: guus.der.kinderen):
Since your last comment in this issue, we've followed up in the mailing list.
To summarize:
bq. I've asked a user that suffers from the problem to run with a slightly
modified version of MINA. The modification logs (and rethrows) any Throwable in
Worker's run(). This caused an Error to be logged! The Error was thrown by an
IoHandler implementation. Full details are available at
[https://igniterealtime.atlassian.net/browse/OF-2367?focusedCommentId=22738]
Note that this problem affects _all_ ThreadPoolExecutors, not just the ordered
one!
I have written various unit tests to illustrate the issue, which are attached
to this issue (note that these are all new classes, except for
{{PriorityThreadPoolExecutorTest.java}}, which expands on a pre-existing
implementation).
The tests in [^PriorityThreadPoolExecutorTest.java]
[^OrderedThreadPoolExecutorTest.java] and
[^UnorderedThreadPoolExecutorTest.java] assert that the {{run()}} method of the
{{Worker}} class in each {{ThreadPoolExecutor}} implementation is susceptible
to this problem, but uses an implementation that bypasses some of the usage
patterns that would normally prevent this issue. These tests prove that the
implementation is weak, without immediately identifying an exploit of that
weakness.
The tests in [^DIRMINA1156Test.java] simulate one of the real-world causes of
the issue: having a {{Session}} that has a {{Handler}} that throws an {{Error}}
(I've included tests that throw checked and unchecked {{Exception}}s, but those
do not trigger the bug).
Removing the 'finally' block that is inside the {{for (;;)}} block of Worker,
and making execution of {{idleWorkers.incrementAndGet();}} happen only when no
{{Throwable}} is thrown, will make all these tests go green. In other words,
replace:
{code:java}
try {
if (session != null) {
runTasks(getSessionTasksQueue(session)); }
} finally {
idleWorkers.incrementAndGet();
}
{code}
with this:
{code:java}
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
idleWorkers.incrementAndGet();
{code}
(a slightly different implementation is needed for PriorityThreadPoolExecutor).
> Inconsistent worker / idleWorker in OrderedThreadPoolExecutor
> -------------------------------------------------------------
>
> Key: DIRMINA-1156
> URL: https://issues.apache.org/jira/browse/DIRMINA-1156
> Project: MINA
> Issue Type: Bug
> Reporter: Guus der Kinderen
> Priority: Critical
> Attachments: DIRMINA1156Test.java,
> OrderedThreadPoolExecutorTest.java, PriorityThreadPoolExecutorTest.java,
> UnorderedThreadPoolExecutorTest.java
>
>
> I stumbled upon this while performing analysis of heap dumps of a JVMs that
> suffered from an issue where no user was able to establish a connection to
> our server software anymore.
>
> Our application uses an ExecutorFilter. The OrderedThreadPoolExecutor of the
> affected server seems to be in an inconsistent state:
> * idleWorkers: 2
> * waitingSessions.count: 2293
> * workers.map.size: 0
> What strikes me as odd is:
> * No workers, while there are sessions waiting to be processed
> * No workers, but a non-zero idle workers count
> Servers that are unaffected by the issue have an idle worker count that is
> equal to the amount of workers (I assume that the snapshots were taken when
> that server was not actively processing data).
> We are using Apache MINA 2.1.3. I have no indication that this problem is or
> is not present in other versions.
> This issue has also been discussed on the Apache MINA users mailinglist,
> starting with this message:
> https://www.mail-archive.com/[email protected]/msg06887.html
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]