You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hc.apache.org by "Nicolas Capponi (Jira)" <ji...@apache.org> on 2021/01/19 17:14:00 UTC
[jira] [Updated] (HTTPCORE-659) Race condition in IOSessionImpl
when setting event
[ https://issues.apache.org/jira/browse/HTTPCORE-659?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Nicolas Capponi updated HTTPCORE-659:
-------------------------------------
Description:
There is a race condition in class {{org.apache.hc.core5.reactor.IOSessionImpl }}when setting an event through the{{ {{setEvent(final int op)}}}} method: there is a check on the session to ensure it is not closed but this check is performed outside the lock that is later acquired to protect the call on the {{SelectionKey}}. If the session is closed just after the check but before getting the lock, then it will throw a {{CancelledKeyException}} when trying to perform the call on the {{SelectionKey}}.
I have implemented an HTTP server that use the reactive library ({{httpcore5-reactive}} component) and execute the requests in a separate thread pool. I was hit by this issue, here is a stack trace (with traces non specific to HttpCore library removed):
{code:java}
CancelledKeyException at SelectionKeyImpl.java:71
SelectionKeyImpl.java:90
IOSessionImpl.java:159
InternalDataChannel.java:324
AbstractHttp1StreamDuplexer.java: 452
ServerHttp1StreamDuplexer.java:136
ServerHttp1StreamHandler.java:94
ReactiveDataProducer.java:108
ReactiveDataProducer.java:100
... traces from publisher implementation using Rx java library...
ReactiveServerExchangeHandler.java:91
ReactiveServerExchangeHandler.java:83
...{code}
I was able to reproduce it almost systematically with unit test from my application (concurrency issues are very sensitive to time). If necessary I can provide a sample {{HttpAsyncServer}} which allows me to reproduce it by introducing a simple {{Thread.sleep(50)}} sequence in {{IOSessionImpl}} code.
There is a simple fix which solves the problem: simply move the check on close status inside the lock-unlock block:
{code:java}
lock.lock();
try {
if (isStatusClosed()) {
return;
}
this.key.interestOps(this.key.interestOps() | op);
} finally {
lock.unlock();
}
{code}
Note that 2 other methods of this class are subject to this problem: {{setEventMask(final int newValue)}} and \{{ clearEvent(final int op) }}. The fix is the same than for the first method.
I can submit a pull request with the fix(es) if you indicate me in which branch I should submit it.
was:
There is a race condition in class {{org.apache.hc.core5.reactor.IOSessionImpl }}when setting an event through the {{setEvent(final int op)}} method: there is a check on the session to ensure it is not closed but this check is performed outside the lock that is later acquired to protect the call on the {{SelectionKey}}. If the session is closed just after the check but before getting the lock, then it will throw a {{CancelledKeyException}} when trying to perform the call on the {{SelectionKey}}.
I have implemented an HTTP server that use the reactive library ({{httpcore5-reactive}} component) and execute the requests in a separate thread pool. I was hit by this issue, here is a stack trace (with traces non specific to HttpCore library removed):
{code:java}
CancelledKeyException at SelectionKeyImpl.java:71
SelectionKeyImpl.java:90
IOSessionImpl.java:159
InternalDataChannel.java:324
AbstractHttp1StreamDuplexer.java: 452
ServerHttp1StreamDuplexer.java:136
ServerHttp1StreamHandler.java:94
ReactiveDataProducer.java:108
ReactiveDataProducer.java:100
... traces from publisher implementation using Rx java library...
ReactiveServerExchangeHandler.java:91
ReactiveServerExchangeHandler.java:83
...{code}
I was able to reproduce it almost systematically with unit test from my application (concurrency issues are very sensitive to time). If necessary I can provide a sample {{HttpAsyncServer}} which allows me to reproduce it by introducing a simple {{Thread.sleep(50)}} sequence in {{IOSessionImpl}} code.
There is a simple fix which solves the problem: simply move the check on close status inside the lock-unlock block:
{code:java}
// lock.lock();
try {
if (isStatusClosed()) {
return;
}
this.key.interestOps(this.key.interestOps() | op);
} finally {
lock.unlock();
}
{code}
Note that 2 other methods of this class are subject to this problem: {{setEventMask(final int newValue)}} and \{{ clearEvent(final int op) }}. The fix is the same than for the first method.
I can submit a pull request with the fix(es) if you indicate me in which branch I should submit it.
> Race condition in IOSessionImpl when setting event
> --------------------------------------------------
>
> Key: HTTPCORE-659
> URL: https://issues.apache.org/jira/browse/HTTPCORE-659
> Project: HttpComponents HttpCore
> Issue Type: Bug
> Components: HttpCore NIO
> Affects Versions: 5.0.3
> Reporter: Nicolas Capponi
> Priority: Major
>
> There is a race condition in class {{org.apache.hc.core5.reactor.IOSessionImpl }}when setting an event through the{{ {{setEvent(final int op)}}}} method: there is a check on the session to ensure it is not closed but this check is performed outside the lock that is later acquired to protect the call on the {{SelectionKey}}. If the session is closed just after the check but before getting the lock, then it will throw a {{CancelledKeyException}} when trying to perform the call on the {{SelectionKey}}.
> I have implemented an HTTP server that use the reactive library ({{httpcore5-reactive}} component) and execute the requests in a separate thread pool. I was hit by this issue, here is a stack trace (with traces non specific to HttpCore library removed):
>
> {code:java}
> CancelledKeyException at SelectionKeyImpl.java:71
> SelectionKeyImpl.java:90
> IOSessionImpl.java:159
> InternalDataChannel.java:324
> AbstractHttp1StreamDuplexer.java: 452
> ServerHttp1StreamDuplexer.java:136
> ServerHttp1StreamHandler.java:94
> ReactiveDataProducer.java:108
> ReactiveDataProducer.java:100
> ... traces from publisher implementation using Rx java library...
> ReactiveServerExchangeHandler.java:91
> ReactiveServerExchangeHandler.java:83
> ...{code}
>
> I was able to reproduce it almost systematically with unit test from my application (concurrency issues are very sensitive to time). If necessary I can provide a sample {{HttpAsyncServer}} which allows me to reproduce it by introducing a simple {{Thread.sleep(50)}} sequence in {{IOSessionImpl}} code.
> There is a simple fix which solves the problem: simply move the check on close status inside the lock-unlock block:
>
> {code:java}
> lock.lock();
> try {
> if (isStatusClosed()) {
> return;
> }
> this.key.interestOps(this.key.interestOps() | op);
> } finally {
> lock.unlock();
> }
> {code}
> Note that 2 other methods of this class are subject to this problem: {{setEventMask(final int newValue)}} and \{{ clearEvent(final int op) }}. The fix is the same than for the first method.
> I can submit a pull request with the fix(es) if you indicate me in which branch I should submit it.
>
>
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@hc.apache.org
For additional commands, e-mail: dev-help@hc.apache.org