[
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, 4:39 PM:
----------------------------------------------------------------------
Since your last comment in this issue, we've followed up in the mailing list.
To summarize:
> 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 exceptions, 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:
```
try {
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
} finally {
idleWorkers.incrementAndGet();
}
```
with this:
```
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
idleWorkers.incrementAndGet();
```
(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:
> 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 exceptions, 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:
```java
try {
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
} finally {
idleWorkers.incrementAndGet();
}
```
with this:
```java
if (session != null) {
runTasks(getSessionTasksQueue(session));
}
idleWorkers.incrementAndGet();
```
(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]
