[jira] [Commented] (DIRMINA-1116) Session Message Order Is Mixed Up Using NioSocketAcceptor

2019-06-06 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16858171#comment-16858171
 ] 

Emmanuel Lecharny commented on DIRMINA-1116:


There is something you can do : check that what is sent by the client is what 
MINA receives. You need a wireshark capture and a dump of the buffer read by 
MINA. Comparing both would show you if the pb is on the client (if both data 
are equals) or in MINA (if there is a difference).

> Session Message Order Is Mixed Up Using NioSocketAcceptor
> -
>
> Key: DIRMINA-1116
> URL: https://issues.apache.org/jira/browse/DIRMINA-1116
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.1.2
> Environment: Windows
>Reporter: Charlie
>Priority: Trivial
>
> The application requires clients to send communications in a specific 
> sequence. 
> For some reason the messages received by a session sometimes get processed 
> out of order by MINA.
> Here is my usage of MINA:
> {code:java}
> //Setup filter chain
> DefaultIoFilterChainBuilder chain;
> acceptor = new NioSocketAcceptor(Runtime.getRuntime().availableProcessors() + 
> 1);
>  chain = acceptor.getFilterChain();
>  chain.addLast("codec", new ProtocolCodecFilter(new 
> ApplicationProtocolCodecFactory()));
>  chain.addLast("authentication", new AuthenticationFilter());
>  chain.addLast("inputVerification", new InputValidatorFilter());
> acceptor.setHandler(handler);{code}
> The messages are already out of order in the protocol decoder filter.
> The client application sends messages using one thread and uses tcp so it 
> can't be sending out of order.
> Is this intended behavior due to the async model?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org



[jira] [Commented] (DIRMINA-1116) Session Message Order Is Mixed Up Using NioSocketAcceptor

2019-06-06 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16858147#comment-16858147
 ] 

Emmanuel Lecharny commented on DIRMINA-1116:


At this point, you have top understand that the server side is just reading 
bytes in the order they have been emitted. MINA does not do any reordering 
whatsoever Once the session is created, it's associated with on single 
{{IoProcessor}} which reads the channel as soon as it gets signaled. In other 
words: if it's garbled on the server, then it's garbled by the client.

Note that on TCP messages may be sliced (ie you can eventually receive an 
entire message one byte by one byte). Get ready for that. That means your 
decoder *must* accumulate data until a full message has been read. You simply 
can't assume that when  you send, say, 4Kb of data, the server will receive 
those 4Kb in one block. It's likely not going to be the case.

> Session Message Order Is Mixed Up Using NioSocketAcceptor
> -
>
> Key: DIRMINA-1116
> URL: https://issues.apache.org/jira/browse/DIRMINA-1116
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.1.2
> Environment: Windows
>Reporter: Charlie
>Priority: Trivial
>
> The application requires clients to send communications in a specific 
> sequence. 
> For some reason the messages received by a session sometimes get processed 
> out of order by MINA.
> Here is my usage of MINA:
> {code:java}
> //Setup filter chain
> DefaultIoFilterChainBuilder chain;
> acceptor = new NioSocketAcceptor(Runtime.getRuntime().availableProcessors() + 
> 1);
>  chain = acceptor.getFilterChain();
>  chain.addLast("codec", new ProtocolCodecFilter(new 
> ApplicationProtocolCodecFactory()));
>  chain.addLast("authentication", new AuthenticationFilter());
>  chain.addLast("inputVerification", new InputValidatorFilter());
> acceptor.setHandler(handler);{code}
> The messages are already out of order in the protocol decoder filter.
> The client application sends messages using one thread and uses tcp so it 
> can't be sending out of order.
> Is this intended behavior due to the async model?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org



[jira] [Commented] (DIRMINA-996) IoSessionRecycler RemoteAddress Collision

2019-06-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16855899#comment-16855899
 ] 

Emmanuel Lecharny commented on DIRMINA-996:
---

Thanks Paul. Yes, same idea : you generate a key based on the local and remote 
address. This is most certainly the easiest way to have it working. The 
Map is probably overkilling.

> IoSessionRecycler RemoteAddress Collision
> -
>
> Key: DIRMINA-996
> URL: https://issues.apache.org/jira/browse/DIRMINA-996
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.7
>Reporter: Flavio Battimo
>Assignee: Jonathan Valliere
>Priority: Critical
> Fix For: 2.1.4
>
>
> When using NioDatagramAcceptor with multiple binded local addresses the 
> IoSessionRecycler is not working as expected.
> If the acceptor has been prefilled with two sessions:
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1000
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1001
> so with same remote address but different local address, when the recycle 
> method of IoSessionRecycler interface is called only remote address is 
> passed. The recycler returns the same IoSession for incoming datagrams on two 
> different local addresses of the same acceptor.
> The IoSessionRecycler should have the local address parameter also in order 
> to find the correct IoSession.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org



[jira] [Commented] (DIRMINA-996) IoSessionRecycler RemoteAddress Collision

2019-06-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16855808#comment-16855808
 ] 

Emmanuel Lecharny commented on DIRMINA-996:
---

The {{ExpiringMap}} could use a {{Map}} as a value, so an 
expiring session would be find doing a fetch from the encapsulating {{Map}} 
then another fetch from the internal {{Map}} to get the {{IoSession}} that 
expired.

Another option would be to create a key that is a concatenation of the local 
and remote address.

> IoSessionRecycler RemoteAddress Collision
> -
>
> Key: DIRMINA-996
> URL: https://issues.apache.org/jira/browse/DIRMINA-996
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.7
>Reporter: Flavio Battimo
>Assignee: Jonathan Valliere
>Priority: Critical
> Fix For: 2.1.4
>
>
> When using NioDatagramAcceptor with multiple binded local addresses the 
> IoSessionRecycler is not working as expected.
> If the acceptor has been prefilled with two sessions:
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1000
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1001
> so with same remote address but different local address, when the recycle 
> method of IoSessionRecycler interface is called only remote address is 
> passed. The recycler returns the same IoSession for incoming datagrams on two 
> different local addresses of the same acceptor.
> The IoSessionRecycler should have the local address parameter also in order 
> to find the correct IoSession.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org



[jira] [Commented] (DIRMINA-1116) Session Message Order Is Mixed Up Using NioSocketAcceptor

2019-06-02 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1116?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16854114#comment-16854114
 ] 

Emmanuel Lecharny commented on DIRMINA-1116:


Otherwise, can you attach a capture of the flow (wireshark) ?

> Session Message Order Is Mixed Up Using NioSocketAcceptor
> -
>
> Key: DIRMINA-1116
> URL: https://issues.apache.org/jira/browse/DIRMINA-1116
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.1.2
> Environment: Windows
>Reporter: Charlie
>Priority: Major
>
> The application requires clients to send communications in a specific 
> sequence. 
> For some reason the messages received by a session sometimes get processed 
> out of order by MINA.
> Here is my usage of MINA:
> {code:java}
> //Setup filter chain
> DefaultIoFilterChainBuilder chain;
> acceptor = new NioSocketAcceptor(Runtime.getRuntime().availableProcessors() + 
> 1);
>  chain = acceptor.getFilterChain();
>  chain.addLast("codec", new ProtocolCodecFilter(new 
> ApplicationProtocolCodecFactory()));
>  chain.addLast("authentication", new AuthenticationFilter());
>  chain.addLast("inputVerification", new InputValidatorFilter());
> acceptor.setHandler(handler);{code}
> The messages are already out of order in the protocol decoder filter.
> The client application sends messages using one thread and uses tcp so it 
> can't be sending out of order.
> Is this intended behavior due to the async model?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

-
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org



[jira] [Updated] (DIRMINA-1105) SSLHandler buffer handling

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1105:
---
Fix Version/s: (was: 2.1.3)
   2.1.4

> SSLHandler buffer handling
> --
>
> Key: DIRMINA-1105
> URL: https://issues.apache.org/jira/browse/DIRMINA-1105
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21, 2.1.1
>Reporter: Emmanuel Lecharny
>Assignee: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.4
>
>
> The {{SSLEngine.wrap()}} method requires the provided buffer to be 'big 
> enough' to contain any kind of *SSL/TLS* message. That means 16921 bytes. The 
> way it's implemented is that we allocate such a buffer every time we need to 
> call the {{wrap}}  method, then we copy the result into a smaller buffer that 
> is injected into thee write queue.
> This is quite ineficient. It would rather be a better idea to use a Thread 
> Local Storage buffer when calling the {{wrap}}  method, and copy the content 
> into a temporary buffer.
> Another optimization could be to concatenate the successive calls to the 
> {{wrap}} method into a single  buffer, that will be sent in one shot (it's 
> frequent that more than one call to {{wrap}} is needed during the handshake).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-996) IoSessionRecycler RemoteAddress Collision

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-996:
--
Fix Version/s: (was: 2.1.3)
   2.1.4

> IoSessionRecycler RemoteAddress Collision
> -
>
> Key: DIRMINA-996
> URL: https://issues.apache.org/jira/browse/DIRMINA-996
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.7
>Reporter: Flavio Battimo
>Assignee: Jonathan Valliere
>Priority: Critical
> Fix For: 2.1.4
>
>
> When using NioDatagramAcceptor with multiple binded local addresses the 
> IoSessionRecycler is not working as expected.
> If the acceptor has been prefilled with two sessions:
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1000
> remoteaddr=192.168.1.10:2001 localaddr=192.168.1.20:1001
> so with same remote address but different local address, when the recycle 
> method of IoSessionRecycler interface is called only remote address is 
> passed. The recycler returns the same IoSession for incoming datagrams on two 
> different local addresses of the same acceptor.
> The IoSessionRecycler should have the local address parameter also in order 
> to find the correct IoSession.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1064) Implement cipher suites preference flag introduced in JDK 8

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1064:
---
Fix Version/s: (was: 2.1.3)
   2.1.4
   2.0.23

> Implement cipher suites preference flag introduced in JDK 8
> ---
>
> Key: DIRMINA-1064
> URL: https://issues.apache.org/jira/browse/DIRMINA-1064
> Project: MINA
>  Issue Type: Improvement
>  Components: SSL
>Affects Versions: 2.0.16
> Environment: N/A - this is general setting not dependent on 
> OS/Hardware
>Reporter: Grzegorz Lech
>Assignee: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.0.23, 2.1.4
>
>
> Oracle Java 8 introduced a method in javax.net.ssl.SSLParameters named 
> setUseCipherSuitesOrder(boolean). The Mina classes are not able to use this 
> parameter. The proposition is to make changes in 
> org.apache.mina.filter.ssl.SslFilter to create new setting and pass it to the 
> class named:org.apache.mina.filter.ssl.SslHandler  and method named: init() 
> to make this setting configurable (expose it in MINA classes).
> What more, the MINA jar should be compiled in 1.8 to use this param. 



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1035) HttpServerDecoder not handling colons in header values

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1035:
---
Fix Version/s: (was: 2.1.3)
   2.1.4

> HttpServerDecoder not handling colons in header values
> --
>
> Key: DIRMINA-1035
> URL: https://issues.apache.org/jira/browse/DIRMINA-1035
> Project: MINA
>  Issue Type: Bug
>  Components: Protocol - HTTP
>Affects Versions: 2.0.13
>Reporter: Mark Ford
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.0.23, 2.1.4
>
> Attachments: DIRMINA-1035.patch
>
>
> The HttpServerDecoder is splitting the HTTP header name/value on a colon via 
> a regular expression. The problem is that some HTTP headers from applications 
> contain colons in their values. The correct behavior would be to split on the 
> *first* colon and not every colon in the header name/value pair line.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1014) SocketAcceptor doesn't unbind correctly

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1014:
---
Fix Version/s: (was: 2.1.3)
   2.1.4

> SocketAcceptor doesn't unbind correctly
> ---
>
> Key: DIRMINA-1014
> URL: https://issues.apache.org/jira/browse/DIRMINA-1014
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.9
> Environment: Ubuntu 12.04.5 x64
> Oracle JDK 1.6.0_45
> Mina 2.0.9
>Reporter: Nipun Jawalkar
>Assignee: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.0.23, 2.1.4
>
>
> When the AbstractIoAcceptor binds a socket, it adds an entry to the 
> "boundAddresses" set to keep track of that socket.
> In my case, I am passing a SocketAddress initialized with a specific port, 
> and "null" for host (I want to listen on that port on all interfaces on the 
> the local machine, i.e 0.0.0.0:). This InetSocketAddress instance has an 
> Inet4Address in it when created.
> When the Acceptor is given this v4 SocketAddress to bind, it creates a 
> ServerSocketChannel, and then calls 
> ServerSocketChannel.socket().getLocalSocketAddress() to retrieve a 
> SocketAddress. This is the value that is then put in the "boundAddresses" 
> collection.
> The problem is that the ServerSocketChannel.socket().getLocalSocketAddress() 
> returns a SocketAddress with an Inet6Address value in it 
> (0.0.0.0.0.0.0.0:), even though the SocketAcceptor.bind() was given a v4 
> address. So when I try to unbind that socket (using the same v4 address I 
> used to do the bind() call), it fails because the "boundAddresses" set has 
> the v6 version in it, and "Inet6Address.equals(Inet4Address) is false".
> Basically:
> 1. I call NioSocketAcceptor.bind() on a v4 SocketAddress
> 2. A ServerSocketChannel is created for that v4 address.
> 3. The ServerSocketChannel.socket().getLocalSocketAddress() is called, which 
> returns a v6 SocketAddress (which is the equivalent of the v4 SocketAddress 
> given)
> 4. NioSocketAcceptor puts the v6 address in its "boundAddresses" list.
> 5. When I ask to unbind the same v4 address, it fails because the 
> "boundAddresses" list only contains a v6 address.
> Suggested Fix:
> 1. Make the "boundAddresses" field a Map. The 
> keys should be the SocketAddress parameter passed to the bind() call, the 
> values should be the ServerSocketChannel.socket().getLocalSocketAddress().
> 2. When unbind(a) is called, check "boundAddresses.hasKey(a)", and if true, 
> then unbind "boundAddresses.get(a)".
> This makes it consistent for the client; they can use the same instance of 
> SocketAddress to bind and unbind reliably.
> This will match Mina v1.x version, where the SocketAddress passed to the 
> bind() method could be used to also unbind.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-29 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1113:
---
Fix Version/s: (was: 2.1.3)
   2.1.4

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.4
>
> Attachments: PriorityThreadPoolExecutor-failure.diff
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-29 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16850828#comment-16850828
 ] 

Emmanuel Lecharny commented on DIRMINA-1113:


The test is marked as {{@Ignored}} atm. 

Guus, if you can tell us a bit more about the logic behind this test, that 
would help a lot.

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.4
>
> Attachments: PriorityThreadPoolExecutor-failure.diff
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-25 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-.

Resolution: Fixed

Fixed.

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
> Attachments: image-2019-05-21-11-37-41-931.png
>
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-24 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847944#comment-16847944
 ] 

Emmanuel Lecharny commented on DIRMINA-1113:


It seems that the test is faulty. Here is the {{waitingSessions}} queue, as it 
is created :

{noformat}
New session entrey (0x0002: mina dummy, server, ? => ?)
2
New session entrey (0x0003: mina dummy, server, ? => ?)
Session removed:(0x0002: mina dummy, server, ? => ?)
2, 3
New session entrey (0x0004: mina dummy, server, ? => ?)
3, 4
New session entrey (0x0005: mina dummy, server, ? => ?)
session1 preferred
5, 4, 3
New session entrey (0x0006: mina dummy, server, ? => ?)
5, 4, 3, 6
New session entrey (0x0007: mina dummy, server, ? => ?)
5, 4, 3, 6, 7
New session entrey (0x0008: mina dummy, server, ? => ?)
Session removed:(0x0005: mina dummy, server, ? => ?)
5, 4, 3, 6, 7, 8
New session entrey (0x0009: mina dummy, server, ? => ?)
3, 4, 8, 6, 7, 9
New session entrey (0x000A: mina dummy, server, ? => ?)
3, 4, 8, 6, 7, 9, 10
New session entrey (0x0001: mina dummy, server, ? => ?)
Session removed:(0x0003: mina dummy, server, ? => ?)
3, 4, 8, 6, 7, 9, 10, 1
New session entrey (0x0002: mina dummy, server, ? => ?)
4, 6, 8, 1, 7, 9, 10, 2
New session entrey (0x0005: mina dummy, server, ? => ?)
session1 preferred
session1 preferred
session1 preferred
5, 4, 8, 6, 7, 9, 10, 2, 1
{noformat}

The prefered session is session 5, which is removed at some point, then 
re-added. In this case, the session {{lastActivity}} value is changing, and it 
appears that when trying later on to check all the sessions, session 3 is 
actually older than the new session 5.

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.3
>
> Attachments: PriorityThreadPoolExecutor-failure.diff
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-24 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1113:
---
Attachment: PriorityThreadPoolExecutor-failure.diff

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.3
>
> Attachments: PriorityThreadPoolExecutor-failure.diff
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-24 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847830#comment-16847830
 ] 

Emmanuel Lecharny commented on DIRMINA-1113:


The patch for DIRMINA-1110 does not fix the problem.

I have attached a patch that makes the test failing 100%. This is a non active 
code addition, it just prints out some info, but doing so, it seems to 
perturbate the ordered queue.

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.3
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-24 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1113:
---
Fix Version/s: 2.1.3

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.3
>
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1110) Ordered Executors concurrency

2019-05-24 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1110?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847557#comment-16847557
 ] 

Emmanuel Lecharny commented on DIRMINA-1110:


Patch applied in 2.1.X branch.

Many thanks Guus !

(I think it's going to fix DIRMINA-1113)

> 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
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-23 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847111#comment-16847111
 ] 

Emmanuel Lecharny commented on DIRMINA-1113:


No, this is not the cause.

If I print what happens in the {{UnfairComparator.compare()}} method, the test 
will fail if the second parameter is the preferred session.

{code:java}
@Override
public int compare(IoSession o1, IoSession o2) {
if (o1 == preferred) {
System.out.println( "O1 prefered" );
return -1;
}

if (o2 == preferred) {
System.out.println( "O2 prefered" );
return 1;
}

return 0;
}
{code}

generally prints :

{noformat}
O1 prefered
O1 prefered
{noformat}

and the test succeed, but when it prints :
{noformat}
O1 prefered
O1 prefered
O2 prefered
{noformat}

the test fails in this case. I have no idea why it happens (and it's quite 
rare)...


> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-23 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1113?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16847045#comment-16847045
 ] 

Emmanuel Lecharny commented on DIRMINA-1113:


I think there is a possible collision as we compare two milliseconds which are 
generated using {{System.currentTimeMillis()}}, and we have a 20ms wait. The 
thing is that {{System.currentTimeMillis()}} accuracy is more or less 10ms (it 
depends on the OS though), so waiting for 20 ms offer a slight opportunity to 
have both time to be equal.

> Random failure on PriorityThreadPoolExecutorTest.testPrioritisation
> ---
>
> Key: DIRMINA-1113
> URL: https://issues.apache.org/jira/browse/DIRMINA-1113
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.22, 2.1.2
>Reporter: Emmanuel Lecharny
>Priority: Major
>
> We have random failure when running {{mvn clean install}} :
> {noformat}
> ...
> [ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
> 38.815 s <<< FAILURE! - in 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
> [ERROR] 
> testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
>   Time elapsed: 38.815 s  <<< FAILURE!
> java.lang.AssertionError: All other sessions should have finished later than 
> the preferred session (but at least one did not).
>   at 
> org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)
> [INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 
> s - in org.apache.mina.filter.keepalive.KeepAliveFilterTest
> [INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 
> s - in org.apache.mina.filter.logging.MdcInjectionFilterTest
> [INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 
> s - in org.apache.mina.filter.buffer.BufferedWriteFilterTest
> [INFO] 
> [INFO] Results:
> [INFO] 
> [ERROR] Failures: 
> [ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
> sessions should have finished later than the preferred session (but at least 
> one did not).
> [INFO] 
> [ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
> ...
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-22 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845663#comment-16845663
 ] 

Emmanuel Lecharny commented on DIRMINA-:


I have commented the {{buf.reset()}} line.

Guus, could you give 2.1 a try ? (you'll need to grab the 2.1.X branch and 
build it).

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Attachments: image-2019-05-21-11-37-41-931.png
>
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (DIRMINA-1113) Random failure on PriorityThreadPoolExecutorTest.testPrioritisation

2019-05-22 Thread Emmanuel Lecharny (JIRA)
Emmanuel Lecharny created DIRMINA-1113:
--

 Summary: Random failure on 
PriorityThreadPoolExecutorTest.testPrioritisation
 Key: DIRMINA-1113
 URL: https://issues.apache.org/jira/browse/DIRMINA-1113
 Project: MINA
  Issue Type: Bug
Affects Versions: 2.1.2, 2.0.22
Reporter: Emmanuel Lecharny


We have random failure when running {{mvn clean install}} :

{noformat}
...
[ERROR] Tests run: 5, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 38.815 
s <<< FAILURE! - in 
org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest
[ERROR] 
testPrioritisation(org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest)
  Time elapsed: 38.815 s  <<< FAILURE!
java.lang.AssertionError: All other sessions should have finished later than 
the preferred session (but at least one did not).
at 
org.apache.mina.filter.executor.PriorityThreadPoolExecutorTest.testPrioritisation(PriorityThreadPoolExecutorTest.java:214)

[INFO] Running org.apache.mina.filter.keepalive.KeepAliveFilterTest
[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.02 s 
- in org.apache.mina.filter.keepalive.KeepAliveFilterTest
[INFO] Running org.apache.mina.filter.logging.MdcInjectionFilterTest
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.118 s 
- in org.apache.mina.filter.logging.MdcInjectionFilterTest
[INFO] Running org.apache.mina.filter.buffer.BufferedWriteFilterTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 s 
- in org.apache.mina.filter.buffer.BufferedWriteFilterTest
[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   PriorityThreadPoolExecutorTest.testPrioritisation:214 All other 
sessions should have finished later than the preferred session (but at least 
one did not).
[INFO] 
[ERROR] Tests run: 214, Failures: 1, Errors: 0, Skipped: 8
...
{noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-21 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16845261#comment-16845261
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Can we backport the fix to 2.0 ?

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-21 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844890#comment-16844890
 ] 

Emmanuel Lecharny commented on DIRMINA-:


We might updated the written bytes counter directly in the 
{{clearWriteRequestQueue()}} method, assuming we know how many bytes have 
already been  written.

In any case, if the stats are wrong but the code works (ie does not eat 
100%CPU), I guess it's acceptable for a moment, until we come to a rational 
view on how to correct the stats.

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Attachments: image-2019-05-21-11-37-41-931.png
>
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-21 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844847#comment-16844847
 ] 

Emmanuel Lecharny commented on DIRMINA-:


Ok, after some history digging, I find out why {{buff.reset()}} was called. 
This is a 11 years old addition : 
[|http://svn.apache.org/viewvc?view=revision=592218].

The rational, AFAICT, was to be sure the stats about the number of written 
bytes is correct. We should not count the non-written bytes still present in a 
buffer when the session is disposed.

Calling {{clear()}} instead of {{ereset()}} - which only work when a {{mark()}} 
has been set - seems the right thing to do. Note that with {{2.0.X}} we don't 
have this issue because we call {{mark}}, something we don't do anymore with 
{{2.1.X}}.

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Attachments: image-2019-05-21-11-37-41-931.png
>
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-21 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844683#comment-16844683
 ] 

Emmanuel Lecharny commented on DIRMINA-:


Do you have a link to the OpenFire issue ?

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Attachments: image-2019-05-21-11-37-41-931.png
>
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-21 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16844611#comment-16844611
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Great !

Jonathan, I let you merge the branch. We probably want to backport the change 
to the 2.0.X branch too.

Let me know and I'll kick a release when done.

Thanks a lot  !

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-20 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16843823#comment-16843823
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Guus,

I have checked Jonathan's patch on  my machine (Mac OSX with  Java 1.8.0_192, 
it works like a charm.

Would you give it a try ?

Thanks !

{noformat}
$ git clone --single-branch http://gitbox.apache.org/repos/asf/mina.git -b 
DIRMINA-1107 mina-DIRMINA-1107
$ cd  mina-DIRMINA-1107/
$ git status
On branch DIRMINA-1107
Your branch is up to date with 'origin/DIRMINA-1107'.
$ mvn  clean install -Pserial
...
[INFO] 
[INFO] Reactor Summary for Apache MINA 2.1.3-SNAPSHOT:
[INFO] 
[INFO] Apache MINA  SUCCESS [  1.515 s]
[INFO] Apache MINA Legal .. SUCCESS [  1.327 s]
[INFO] Apache MINA Core ... SUCCESS [02:00 min]
[INFO] Apache MINA APR Transport .. SUCCESS [  0.453 s]
[INFO] Apache MINA Compression Filter . SUCCESS [  1.049 s]
[INFO] Apache MINA State Machine .. SUCCESS [  1.336 s]
[INFO] Apache MINA JavaBeans Integration .. SUCCESS [  1.010 s]
[INFO] Apache MINA XBean Integration .. SUCCESS [  1.692 s]
[INFO] Apache MINA OGNL Integration ... SUCCESS [  0.281 s]
[INFO] Apache MINA JMX Integration  SUCCESS [  0.290 s]
[INFO] Apache MINA Examples ... SUCCESS [  3.038 s]
[INFO] Apache MINA HTTP client and server codec ... SUCCESS [  0.951 s]
[INFO] Apache MINA Serial Communication support ... SUCCESS [  0.305 s]
[INFO] 
[INFO] BUILD SUCCESS
[INFO] 
[INFO] Total time:  02:14 min
[INFO] Finished at: 2019-05-20T11:46:13+02:00
[INFO] 
{noformat}


> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // 

[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-17 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842300#comment-16842300
 ] 

Emmanuel Lecharny commented on DIRMINA-:


Running {{top -H -p }} and {{jstack }} could help finding 
which thread is consuming CPU.

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-17 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842296#comment-16842296
 ] 

Emmanuel Lecharny commented on DIRMINA-:


Also we would need the complete stack trace.  The part that is shown might be a 
red-herring: just because a thread is marked RUNNABLE does not mean it's 
actually doing something. When the call is to a native method, the JVM can't 
know if the method is doing something, so by default tag it as RUNNABLE.

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1111) 100% CPU (epoll bug) on 2.1.x, Linux only

2019-05-17 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842288#comment-16842288
 ] 

Emmanuel Lecharny commented on DIRMINA-:


Hmmm, I don't think so. There is not so many differences between 2.0 and 2.1 
branches, and we haven't touched this part. There must be something else.

We have a mechanism in both version to catch such an issue, which drops the 
selector, recreates one, and re-register all the channels. It's suppose to do 
the trick. Now, there is a know JDK issue that has never been fixed (see 
https://github.com/netty/netty/issues/327).

What is the JVM in use ?

> 100% CPU (epoll bug) on 2.1.x, Linux only
> -
>
> Key: DIRMINA-
> URL: https://issues.apache.org/jira/browse/DIRMINA-
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
>
> We're getting reports 
> [reports|https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13]
>  of a bug that causes 100% CPU usage on Linux (the problem does not occur on 
> Windows). 
> This problem occurs in 2.1.2. but does _not_ occur in 2.0.21.
> Is this a regression of the epoll selector bug in DIRMINA-678 ?
> A stack trace of one of the spinning threads:
> {code}"NioProcessor-3" #55 prio=5 os_prio=0 tid=0x7f0408002000 nid=0x2a6a 
> runnable [0x7f0476dee000]
>java.lang.Thread.State: RUNNABLE
>   at sun.nio.ch.EPollArrayWrapper.epollWait(Native Method)
>   at sun.nio.ch.EPollArrayWrapper.poll(EPollArrayWrapper.java:269)
>   at sun.nio.ch.EPollSelectorImpl.doSelect(EPollSelectorImpl.java:93)
>   at sun.nio.ch.SelectorImpl.lockAndDoSelect(SelectorImpl.java:86)
>   - locked <0x0004c486b748> (a sun.nio.ch.Util$3)
>   - locked <0x0004c486b738> (a java.util.Collections$UnmodifiableSet)
>   - locked <0x0004c420ccb0> (a sun.nio.ch.EPollSelectorImpl)
>   at sun.nio.ch.SelectorImpl.select(SelectorImpl.java:97)
>   at 
> org.apache.mina.transport.socket.nio.NioProcessor.select(NioProcessor.java:112)
>   at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:616)
>   at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
>   at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
>   at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
>   at java.lang.Thread.run(Thread.java:748)
>Locked ownable synchronizers:
>   - <0x0004c4d03530> (a 
> java.util.concurrent.ThreadPoolExecutor$Worker){code}
> More info is available at 
> https://discourse.igniterealtime.org/t/openfire-4-3-2-cpu-goes-at-100-after-a-few-hours-on-linux/85119/13



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-17 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842227#comment-16842227
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Continuing my archeocodology, here are the situations where it's possible to 
write a message in the IoHandler while the handshake has not been completed :
o for a server: just when the session is created, and if the {{SSLFilter}} is 
added to the session's chain, and if the handshake is started immediately, then 
the server will expect the next received message to be a {{CLIENT_HELLO}}. 
However, after having injected the filter in the chain, and created the 
{{SslHandler}} instance, and having started the handshake in server mode, then 
a {{sessionCreated}} followed by a {{sessionOpened}} events are called, which 
potentially gives the application to call a {{session.write(message)}}. 
Obviously, this message should *not* go through the {{SSLFilter}} before the 
handshake has been completed, it must be enqueued, waiting for the handshake to 
complete (or fail).
o If the handshake takes a while, and if the session idling detection is 
activated (every second), the it's possible that the {{IoHandler}} gets called 
through the {{sessionIdling}} event, giving the opportunity for the application 
to write a message (ok, that is a remote possibility)

Just writing that down for the sake of documentation...

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go 

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-17 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16842072#comment-16842072
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Forget about leveraging the {{event}} method: as it is initiated in the 
{{SslFilter}}, it won't be available in this very filter...

The big issue I have atm is how to ensure that the messages we have enqueued 
will be flushed when and only when the handshake have been completed, and how 
to guarantee that no new message is going to be enqueued while we are flushing 
them. This can typically happen when we start flushing the existing pending 
messages, while some thread is writing a new message: we must wait until all 
the existing messages in the queue have been written. Also we don't want to 
keep this queue forever: once the handshake has been completed, and the queue 
flushed, it is not useful anymore.

Actually, we need two barriers:
 * one for messages that are produced *before* the handshake has started but 
*after* the {{SslFilter}} has been added into the chain (this is only something 
to deal with when we inject the filter into an active session - like when using 
startTLS -)
 * one for messages that are produced during the handshake

Between those two barriers, all messages are enqueued, after those two 
barriers, no message is enqueued. And the tricky part: while the queue is 
flushed, no message should be added into the queue, but we must not be flushed 
either (to guarantee the messages' order):
{noformat}
[entry barrier]---[exit 
barrier]--[queue flushed]
|<   messages are enqueued>|< messages are 
waiting >|
{noformat}

Now to find a way to code that...

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code 

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841505#comment-16841505
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


FTR, I just deleted a comment and an attached file, as I realized 5 mins after 
having pushed them I was wrong.

The *only* reason we have a write queue is for when we have messages to be 
written while the handshake is not completed: we need to wait and keep those 
messages. My deleted comment was that we can bypass the 
{{SslHandler.scheduleFilterWrite()}} and push the writes to the next filter. 
That works, assuming no messages are written while the handshake is being 
processed (this is possibly what could happen when using {{startTLS}}).

We can leverage the newly added {{event}} in {{2.1}} to start flushing those 
pending messages when the session has been secured, but in {{2.0}} it's going a 
bit more complex.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1107:
---
Attachment: (was: ssl.diff)

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Issue Comment Deleted] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1107:
---
Comment: was deleted

(was: So here is my first proposed change in {{SslFilter}} (see attachement) 
that directly pushes writes to the next filter (aka {{HeadFilter}}) without 
having to use the {{SslHander.flushScheduledEvents()}} method (I haven't yet 
modified the {{SslHandler}} class to get rid of the write queue, but will do 
soon). Tests are passing green.

More to come.)

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1107:
---
Attachment: ssl.diff

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
> Attachments: ssl.diff
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841490#comment-16841490
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


So here is my first proposed change in {{SslFilter}} (see attachement) that 
directly pushes writes to the next filter (aka {{HeadFilter}}) without having 
to use the {{SslHander.flushScheduledEvents()}} method (I haven't yet modified 
the {{SslHandler}} class to get rid of the write queue, but will do soon). 
Tests are passing green.

More to come.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
> Attachments: ssl.diff
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841096#comment-16841096
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Don't worry, we hijacked the thread devising about some future changes. The fix 
will be applied in the existing branches, without breaking the APIs.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-16 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16841071#comment-16841071
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Agreed. In any case, that would be quite a disruptive change, and can't be 
injected in 2.0 or 2.1.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840575#comment-16840575
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


It should never be a filter, that's for sure. Rather than integrating it in the 
session, I would put it in the IoProcessor, as close as possible to the channel.

Why would you suggest to add this in the session ? (just curious)

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840518#comment-16840518
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


We could actually write the fixed size buffer into the {{IoProcessor}} queue, 
and wait for the {{messageSent}} event in the {{SslFilter}} to write the next 
buffer. that would work, but not when having an executor in the chain (except 
if we enqueue the messages to be encrypted in a {{SslFilter}} queue.

For a 3.0 version, this is clearly an option.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840433#comment-16840433
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Side note: The way encrypted messages are handled is a bit mad. Consider a BIG 
message (say, 1Mb). SSL does not allow messages bigger than roughly 16kb (2^14 
minus header, MAC and padding. Except for M$, of course, which allows up to 
32kb messages...). In any case, we will start with a 16Kb buffer, call encrypt 
again and again until the full source has been read and encrypted, increasing 
the buffer as needed. This buffer will contain *many* TLS {{APPLICATION_DATA}} 
records (starting with {{0x17}}). 

We could perfectly chose to always use a fixed size buffer, and once full, send 
it to the remote peer, avoiding the allocation of a crazy big buffer which will 
be discarded when done. The only problem is that we would need to remember when 
we have fully written the source, inorder to properly send the {{messageSent}} 
event.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1108) ThreadPoolExecutors should terminate faster

2019-05-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1108?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840420#comment-16840420
 ] 

Emmanuel Lecharny commented on DIRMINA-1108:


There is a 30 seconds keepalive parameter that explains why it takes 20 second 
to be shutdown. Have you try calling the shutdownNow()}} method ?

> ThreadPoolExecutors should terminate faster
> ---
>
> Key: DIRMINA-1108
> URL: https://issues.apache.org/jira/browse/DIRMINA-1108
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Minor
>
> I've observed that the termination of 
> {{org.apache.mina.filter.executor.PriorityThreadPoolExecutor}} takes another 
> 30 seconds after the last task was executed. More specifically, it takes 30 
> seconds "to long" before 
> {{org.apache.mina.filter.executor.PriorityThreadPoolExecutor#awaitTermination}}
>  returns {{true}}.
> It is likely that this issue also applies to 
> {{org.apache.mina.filter.executor.OrderedThreadPoolExecutor}}, and maybe 
> {{org.apache.mina.filter.executor.UnorderedThreadPoolExecutor}}, as a lot of 
> code is shared between these implementations. I did not explicitly verify 
> these implementations though.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16840413#comment-16840413
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Ok, after having analysed the {{filterWrite}} code a bit more, I don't see any 
reason to differ the writing of messages either. Actually, we should never use 
a queue to stack messages.

All of this madness comes from a 'fix' applied 12 years ago (see 
[DIRMINA-241|https://issues.apache.org/jira/browse/DIRMINA-241]).

So, yes, we may have multiple writes done on a session by different threads 
when using an executor in the chain, but that should be handled properly with 
the {{synchronized(sslHandler}}} in the {{SslFilter}}.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-14 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16839462#comment-16839462
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


My point, exactly. We should have the {{SSLEngine}} process incoming messages 
immediately, and once decrypted, if they are application messages, being pushed 
into the filter chain immediately too (the special case is for handshake 
messages, because we don't push them anywhere, they are consumed by the 
SslHandler).

At this point, we are doing archeocodology :-)

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-14 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16839167#comment-16839167
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


What I mean is that the {{messageReceivedEventQueue}} is only fed by the thread 
that read data from the channel (ie the {{IoProcessor}} thread). There is no 
other thread that interact with it, and there is no reason to postpone the 
propagation of the message to the chain. In other words, there is no reason to 
process those messages in the {{flushScheduledEvents}} method.

The tricky part is to handle properly the handshake messages which should *not* 
be propagated to the chain.

Anyway, that leave the other queue alone, and we can now safely process it in 
the {{flushScheduledEvents}} method without any interaction with the read 
messages.

I have to do some experiment around this idea.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838939#comment-16838939
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


https://tools.ietf.org/html/rfcThe big problem with 8446, 4.1.2, "Because TLS 
1.3 forbids renegotiation, if a server has negotiated TLS 1.3 and receives a 
ClientHello at any other time, it MUST terminate the connection with an 
"unexpected_message" alert."

The big issue with renegotiation is that you can DOS a server with it quite 
easily.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838921#comment-16838921
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Key renegotiation (which has been removed in 1.3) starts like a handshake, with 
a {{ClientHello}} message, over the encrypted layer. This is a very specific 
use case, and should be handled as is. ATM, we don't have implemented it in 
MINA, AFAICT.

What are the control functions you are mentioning ?

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838918#comment-16838918
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


@Jonathan what for ? Once the HS is completed, if you receive something from a 
remote peer, you won't have to write anything back. The only exception is for 
the Handshake protocol.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838914#comment-16838914
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Regarding the multi-threading aspect:
* Incoming messages are handled by one single thread. A session is always 
associated with this thread when created, so we are always safe.
* outgoing messages are a bit different, because we may have an executor in the 
chain. In this case, a session might call a write in many different threads, 
and that must be handled properly. In any case, once the data have been 
encrypted, and pushed into the {{IoProcessor}} queue (always the same), then it 
will be written by a single thread, the {{IoProcessor}} thread.

So we have to consider protecting the {{SSL}} filter against code that has an 
executor. The {{flushScheduleEvents()}} method, which is called by the 
{{startTls}}, stopTls}}, {{messageReceived}}, {{filterWrite}}, {{filterClose}} 
and {{initiateHandshake}} methods, should first propagate the received messages 
- because it's synchronous - and then process all the messages to be written, 
in a protected section. 

Anyway, it's late, I need to rest and review the code again tomorrow or later 
on, to see if I'm right :-)

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by 

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838898#comment-16838898
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


There are a few bad things done. In the {{SslFilter.messageReceived()}} method, 
we push any incoming message if the {{Ssl}} layer has not yet been installed 
fully :

{code:java}
synchronized (sslHandler) {
if (!isSslStarted(session) && sslHandler.isInboundDone()) {
// The SSL session must be established first before we
// can push data to the application. Store the incoming
// data into a queue for a later processing
sslHandler.scheduleMessageReceived(nextFilter, message);
} else {
{code}

It does not make a lot of sense. There is absolutely no way the {{SslFilter}} 
{{messageReceived}} method could be called if the filter has not been set 
fully. The check done is for the case where the {{SSL}} layer has been closed, 
but in this very case, what's the point in passing the received message to the 
{{IoHandler}} ?

Otherwise, incoming messages are processed in the 
{{SslHandler.messageReceived}} method, and the message will be uncrypted, then 
pushed into the queue. The bizarre thing is that we do write encrypted data 
during this call :

{code:java}
private void handleSslData(NextFilter nextFilter, SslHandler sslHandler) 
throws SSLException {
...
// Write encrypted data to be written (if any)
sslHandler.writeNetBuffer(nextFilter);

// handle app. data read (if any)
handleAppDataRead(nextFilter, sslHandler);
}
{code}

I don't see any reason to do so at this point. Why should we write *anything* 
back to the remote peer when we are processing an incoming message ?

This part of the code needs a serious review, IMHO.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> 

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838682#comment-16838682
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


I'd rather have this issue cover both versions.

i'll have a look at this code tonite, but it's a quite convoluted piece of 
code, so I may wait for Jonathan to be able to review it too so that we find a 
proper fix once and for all.

Thanks for the report, Guus !

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.1.2
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838669#comment-16838669
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


The pb is that we have 2 queues, and we have to check both of them at the same 
time. In any case, I also need to review this part of the code.

AFAICT, the idea is to be sure we have pushed all the messages, in the proper 
order (we can't afford 2 threads to poll the queues and flush the messages at 
the same time).

The thing is that I wonder why we need a counter at all.

And BTW disregard the note on the counter that needs to be included in the lock 
section: this is nonsense.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3, 2.0.23
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838647#comment-16838647
 ] 

Emmanuel Lecharny edited comment on DIRMINA-1107 at 5/13/19 3:59 PM:
-

Seems like we should include the counter into the locked part. Actually, I 
don't understand why it's not in this section...  Something like :

{code:java}
void flushScheduledEvents() {
if (sslLock.tryLock()) {
scheduledEvents.incrementAndGet();

try {
do {
while ((event = filterWriteEventQueue.poll()) != null) {
// ...
}

while ((event = messageReceivedEventQueue.poll()) != null){
// ...
}
} while (scheduledEvents.decrementAndGet() > 0);
} finally {
sslLock.unlock();
}
}
}
{code}

At this point, we don't even have to use an {{AtomicInteger}} for the 
{{scheduledEvents}} value.


was (Author: elecharny):
Seems like we should include the counter into the locked part. Actually, I 
don't understand why it's not in this section...  Something like :

{code:java}
void flushScheduledEvents() {
if (sslLock.tryLock()) {
scheduledEvents.incrementAndGet();

try {
do {
while ((event = filterWriteEventQueue.poll()) != null) {
// ...
}

while ((event = messageReceivedEventQueue.poll()) != null){
// ...
}
} while (scheduledEvents.decrementAndGet() > 0);
} finally {
sslLock.unlock();
}
}
}
{code}

At this point, we don't even have to use an AtomicInteger for the 
{{scheduledEvents}} value.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3, 2.0.23
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> 

[jira] [Commented] (DIRMINA-1107) SslHandler flushScheduledEvents race condition, redux

2019-05-13 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1107?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16838647#comment-16838647
 ] 

Emmanuel Lecharny commented on DIRMINA-1107:


Seems like we should include the counter into the locked part. Actually, I 
don't understand why it's not in this section...  Something like :

{code:java}
void flushScheduledEvents() {
if (sslLock.tryLock()) {
scheduledEvents.incrementAndGet();

try {
do {
while ((event = filterWriteEventQueue.poll()) != null) {
// ...
}

while ((event = messageReceivedEventQueue.poll()) != null){
// ...
}
} while (scheduledEvents.decrementAndGet() > 0);
} finally {
sslLock.unlock();
}
}
}
{code}

At this point, we don't even have to use an AtomicInteger for the 
{{scheduledEvents}} value.

> SslHandler flushScheduledEvents race condition, redux
> -
>
> Key: DIRMINA-1107
> URL: https://issues.apache.org/jira/browse/DIRMINA-1107
> Project: MINA
>  Issue Type: Bug
>Reporter: Guus der Kinderen
>Priority: Major
> Fix For: 2.1.3, 2.0.23
>
>
> DIRMINA-1019 addresses a race condition in SslHandler, but unintentionally 
> replaces it with another multithreading issue.
> The fix for DIRMINA-1019 introduces a counter that contains the number of 
> events to be processed. A simplified version of the code is included below.
> {code:java}
> private final AtomicInteger scheduledEvents = new AtomicInteger(0);
> void flushScheduledEvents() {
> scheduledEvents.incrementAndGet();
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (scheduledEvents.decrementAndGet() > 0);
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> We have observed occasions where the value of {{scheduledEvents}} becomes a 
> negative value, while at the same time {{filterWriteEventQueue}} go 
> unprocessed.
> We suspect that this issue is triggered by a concurrency issue caused by the 
> first thread decrementing the counter after a second thread incremented it, 
> but before it attempted to acquire the lock.
> This allows the the first thread to empty the queues, decrementing the 
> counter to zero and release the lock, after which the second thread acquires 
> the lock successfully. Now, the second thread processes any elements in 
> {{filterWriteEventQueue}}, and then processes any elements in 
> {{messageReceivedEventQueue}}. If in between these two checks yet another 
> thread adds a new element to {{filterWriteEventQueue}}, this element can go 
> unprocessed (as the second thread does not loop, since the counter is zero or 
> negative, and the third thread can fail to acquire the lock).
> It's a seemingly unlikely scenario, but we are observing the behavior when 
> our systems are under high load.
> We've applied a code change after which this problem is no longer observed. 
> We've removed the counter, and check on the size of the queues instead:
> {code:java}
> void flushScheduledEvents() {
> if (sslLock.tryLock()) {
> try {
> do {
> while ((event = filterWriteEventQueue.poll()) != null) {
> // ...
> }
> 
> while ((event = messageReceivedEventQueue.poll()) != null){
> // ...
> }
> } while (!filterWriteEventQueue.isEmpty() || 
> !messageReceivedEventQueue.isEmpty());
> } finally {
> sslLock.unlock();
> }
> }
> }{code}
> This code change, as illustrated above, does introduce a new potential 
> problem. Theoretically, an event could be added to the queues and 
> {{flushScheduledEvents}} be called returning {{false}} for 
> {{sslLock.tryLock()}}, exactly after another thread just finished the 
> {{while}} loop, but before releasing the lock. This again would cause events 
> to go unprocessed.
> We've not observed this problem in the wild yet, but we're uncomfortable 
> applying this change as-is.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (FTPSERVER-494) Documentation fix xml syntax: remove extra "

2019-05-08 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved FTPSERVER-494.
-
Resolution: Fixed

Patch applied.

> Documentation fix xml syntax: remove extra "
> 
>
> Key: FTPSERVER-494
> URL: https://issues.apache.org/jira/browse/FTPSERVER-494
> Project: FtpServer
>  Issue Type: Improvement
>Reporter: Thilo Bangert
>Priority: Trivial
>
> {noformat}
> Index: configuration_listeners.mdtext
> ===
> --- configuration_listeners.mdtext  (revision 1858899)
> +++ configuration_listeners.mdtext  (working copy)
> @@ -40,7 +40,7 @@
>  
>  
>  local-port="2323" ip-check="true">
> - external-address="1.2.3.4" />
> + external-address="1.2.3.4" />
>  
>  1.2.3.0/16, 1.2.4.0/16, 1.2.3.4
>  
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FTPSERVER-494) Documentation fix xml syntax: remove extra "

2019-05-08 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/FTPSERVER-494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16835852#comment-16835852
 ] 

Emmanuel Lecharny commented on FTPSERVER-494:
-

Thanks, patch applied !

> Documentation fix xml syntax: remove extra "
> 
>
> Key: FTPSERVER-494
> URL: https://issues.apache.org/jira/browse/FTPSERVER-494
> Project: FtpServer
>  Issue Type: Improvement
>Reporter: Thilo Bangert
>Priority: Trivial
>
> {noformat}
> Index: configuration_listeners.mdtext
> ===
> --- configuration_listeners.mdtext  (revision 1858899)
> +++ configuration_listeners.mdtext  (working copy)
> @@ -40,7 +40,7 @@
>  
>  
>  local-port="2323" ip-check="true">
> - external-address="1.2.3.4" />
> + external-address="1.2.3.4" />
>  
>  1.2.3.0/16, 1.2.4.0/16, 1.2.3.4
>  
> {noformat}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (FTPSERVER-492) Documentation fix for missing attribute name

2019-05-07 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved FTPSERVER-492.
-
Resolution: Fixed

Patch applied (commit http://svn.apache.org/viewvc?rev=1858844=rev)

Many thanks !

> Documentation fix for missing attribute name
> 
>
> Key: FTPSERVER-492
> URL: https://issues.apache.org/jira/browse/FTPSERVER-492
> Project: FtpServer
>  Issue Type: Improvement
>Reporter: Thilo Bangert
>Priority: Trivial
>
> --- configuration_listeners.mdtext (revision 1858830)
> +++ configuration_listeners.mdtext (working copy)
> @@ -76,7 +76,7 @@
>  |---|---|---|---|
>  | file | Path to the key store file | Yes |  |
>  | password | The password for the key store | Yes |  |
> -| Password for the key within the key store | No | Key store password |
> +| key-password | Password for the key within the key store | No | Key store 
> password |
>  | key-alias | Alias of the key to use within the key store | No <| Uses 
> first key found |
>  | type | Key store type | No | JRE key store default type, normally JKS |
>  | algorithm | Key store algorithm | No | SunX509 |



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (FTPSERVER-493) Documentation: remove partial html tag in Markdown

2019-05-07 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved FTPSERVER-493.
-
Resolution: Fixed

Patch applied with commit http://svn.apache.org/viewvc?rev=1858844=rev.

Many thanks !

> Documentation: remove partial html tag in Markdown
> --
>
> Key: FTPSERVER-493
> URL: https://issues.apache.org/jira/browse/FTPSERVER-493
> Project: FtpServer
>  Issue Type: Improvement
>Reporter: Thilo Bangert
>Priority: Trivial
>
> Index: configuration_listeners.mdtext
> ===
> --- configuration_listeners.mdtext  (revision 1858830)
> +++ configuration_listeners.mdtext  (working copy)
> @@ -117,7 +117,7 @@
>  
>  | Attribute | Description | Required | Default value |
>  |---|---|---|---|
> -| class="confluenceTd"> ports| The ports on which the server is allowed to 
> accept passive data connections, see [Configure passive 
> ports](configuration_passive_ports.html) for details| No| Any available port |
> +| ports| The ports on which the server is allowed to accept passive data 
> connections, see [Configure passive ports](configuration_passive_ports.html) 
> for details| No| Any available port |
>  | address| The address on which the server will listen to passive data 
> connections| No| The same address as the control socket for the session |
>  | external-address| The address the server will claim to be listening on in 
> the PASV reply. Useful when the server is behind a NAT firewall and the 
> client sees a different address than the server is using| No|  |
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-05-04 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1101.

   Resolution: Fixed
Fix Version/s: 2.1.2

Fixed for 2.1 branch. Still a problem in. 2.0, but it won't be fixed.

> InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
> Fix For: 2.1.2
>
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)}}
>  {{    at java.lang.Thread.run(Thread.java:748)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-05-04 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1103:
---
Fix Version/s: 2.1.2

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
> Fix For: 2.0.21, 2.1.2
>
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-05-04 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1103.

   Resolution: Fixed
Fix Version/s: 2.0.21

This has been fixed a while ago with commits 
bdc43e6d0953d43171e61204d4791274f6ae22b9  (2.0) and in 2.1

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
> Fix For: 2.0.21
>
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1106) SSL/TLS error when using Apache LDAP API with MINA 2.1.1

2019-04-16 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1106.

Resolution: Fixed

Fixed with commit {color:#00}45ecc54275144866a75107ece7e50741ca526e22{color}

> SSL/TLS error when using Apache LDAP API with MINA 2.1.1
> 
>
> Key: DIRMINA-1106
> URL: https://issues.apache.org/jira/browse/DIRMINA-1106
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.1
>Reporter: Emmanuel Lecharny
>Priority: Critical
> Fix For: 2.1.2
>
>
> When building Apache LDAP API with MINA 2.1.1, SSL/TLS connection returns an 
> error after a timeout has been reached (if the timeout is set to infinite, 
> the API simply stall).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1106) SSL/TLS error when using Apache LDAP API with MINA 2.1.1

2019-04-16 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16818838#comment-16818838
 ] 

Emmanuel Lecharny commented on DIRMINA-1106:


After investigation, this is due to the modified 
{{SslFilter$EncryptedWriteRequest}} class that does not override the 
{{getFuture}} method, returning a future that is not the original one on which 
the API is waiting.

Adding the following method fixes the issue:

{code:java}
/**
 * {@inheritDoc}
 */
@Override
public WriteFuture getFuture() {
return parentRequest.getFuture();
}
{code}

> SSL/TLS error when using Apache LDAP API with MINA 2.1.1
> 
>
> Key: DIRMINA-1106
> URL: https://issues.apache.org/jira/browse/DIRMINA-1106
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.1.1
>Reporter: Emmanuel Lecharny
>Priority: Critical
> Fix For: 2.1.2
>
>
> When building Apache LDAP API with MINA 2.1.1, SSL/TLS connection returns an 
> error after a timeout has been reached (if the timeout is set to infinite, 
> the API simply stall).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (DIRMINA-1106) SSL/TLS error when using Apache LDAP API with MINA 2.1.1

2019-04-16 Thread Emmanuel Lecharny (JIRA)
Emmanuel Lecharny created DIRMINA-1106:
--

 Summary: SSL/TLS error when using Apache LDAP API with MINA 2.1.1
 Key: DIRMINA-1106
 URL: https://issues.apache.org/jira/browse/DIRMINA-1106
 Project: MINA
  Issue Type: Improvement
Affects Versions: 2.1.1
Reporter: Emmanuel Lecharny
 Fix For: 2.1.2


When building Apache LDAP API with MINA 2.1.1, SSL/TLS connection returns an 
error after a timeout has been reached (if the timeout is set to infinite, the 
API simply stall).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1105) SSLHandler buffer handling

2019-04-15 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1105?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16817977#comment-16817977
 ] 

Emmanuel Lecharny commented on DIRMINA-1105:


We can also improve the way we manage incoming {{TLS}} messages, by using a 
thread local storage buffer.

Most of the time, when we call {{SSLEngine.unwrap()}}, the message is complete 
(ie, it contains all what is needed to be unwrapped). Sometime, due to {{TCP}} 
fragmentation, it's not, and we get back a {{BUFFER_UNDERFLOW}}, then we have 
to wait for more bytes to be read, and recall the unwrap method once more, 
until we get the full message.

I suggest to always feed the read data into the thread local storage buffer, 
call {{unwrap}} with that, and if it returns {{BUFFER_UNDERFLOW}}, allocate a 
temporary buffer stored in the session attributes, which will be discarded when 
the unwrapping will be done (or not).

If we don't get too many fragmented buffers, this approach should be beneficial 
in term of memory consumption (no need to allocate a temporary buffer), which 
is a huge potential gain when we have thousands of sessions. Marginally, we may 
have to allocate a buffer for fragmented data, but this cost is already present 
in the current implementation (this buffer is allocated when the filter is 
added into the session's chain).

> SSLHandler buffer handling
> --
>
> Key: DIRMINA-1105
> URL: https://issues.apache.org/jira/browse/DIRMINA-1105
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21, 2.1.1
>Reporter: Emmanuel Lecharny
>Priority: Major
>
> The {{SSLEngine.wrap()}} method requires the provided buffer to be 'big 
> enough' to contain any kind of *SSL/TLS* message. That means 16921 bytes. The 
> way it's implemented is that we allocate such a buffer every time we need to 
> call the {{wrap}}  method, then we copy the result into a smaller buffer that 
> is injected into thee write queue.
> This is quite ineficient. It would rather be a better idea to use a Thread 
> Local Storage buffer when calling the {{wrap}}  method, and copy the content 
> into a temporary buffer.
> Another optimization could be to concatenate the successive calls to the 
> {{wrap}} method into a single  buffer, that will be sent in one shot (it's 
> frequent that more than one call to {{wrap}} is needed during the handshake).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (DIRMINA-1105) SSLHandler buffer handling

2019-04-12 Thread Emmanuel Lecharny (JIRA)
Emmanuel Lecharny created DIRMINA-1105:
--

 Summary: SSLHandler buffer handling
 Key: DIRMINA-1105
 URL: https://issues.apache.org/jira/browse/DIRMINA-1105
 Project: MINA
  Issue Type: Improvement
Affects Versions: 2.0.21, 2.1.1
Reporter: Emmanuel Lecharny


The {{SSLEngine.wrap()}} method requires the provided buffer to be 'big enough' 
to contain any kind of *SSL/TLS* message. That means 16921 bytes. The way it's 
implemented is that we allocate such a buffer every time we need to call the 
{{wrap}}  method, then we copy the result into a smaller buffer that is 
injected into thee write queue.

This is quite ineficient. It would rather be a better idea to use a Thread 
Local Storage buffer when calling the {{wrap}}  method, and copy the content 
into a temporary buffer.

Another optimization could be to concatenate the successive calls to the 
{{wrap}} method into a single  buffer, that will be sent in one shot (it's 
frequent that more than one call to {{wrap}} is needed during the handshake).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1104) IoBufferHexDumper.getHexdump(IoBuffer in, int lengthLimit) does not truncate the output

2019-04-12 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1104:
---
Fix Version/s: 2.1.2
   2.0.22

> IoBufferHexDumper.getHexdump(IoBuffer in, int lengthLimit) does not truncate 
> the output
> ---
>
> Key: DIRMINA-1104
> URL: https://issues.apache.org/jira/browse/DIRMINA-1104
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.21, 2.1.1
>Reporter: Emmanuel Lecharny
>Priority: Trivial
> Fix For: 2.0.22, 2.1.2
>
>
> When the {{lengthLimit}} is below the {{IoBuffer}} limit, the resulting dump 
> should be truncated. It's not. Instead, "..." are appended at the end of the 
> resulting String. I suspect that those "..." are supposed to be added when 
> the result is *really* truncated.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1104) IoBufferHexDumper.getHexdump(IoBuffer in, int lengthLimit) does not truncate the output

2019-04-12 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1104:
---
Affects Version/s: (was: 2.1.0)
   2.1.1

> IoBufferHexDumper.getHexdump(IoBuffer in, int lengthLimit) does not truncate 
> the output
> ---
>
> Key: DIRMINA-1104
> URL: https://issues.apache.org/jira/browse/DIRMINA-1104
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.21, 2.1.1
>Reporter: Emmanuel Lecharny
>Priority: Trivial
>
> When the {{lengthLimit}} is below the {{IoBuffer}} limit, the resulting dump 
> should be truncated. It's not. Instead, "..." are appended at the end of the 
> resulting String. I suspect that those "..." are supposed to be added when 
> the result is *really* truncated.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1095) Seems like the management f UDP sessions is really unneficient

2019-04-12 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1095:
---
Fix Version/s: (was: 2.1.0)
   2.1.1

> Seems like the management f UDP sessions is really unneficient
> --
>
> Key: DIRMINA-1095
> URL: https://issues.apache.org/jira/browse/DIRMINA-1095
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.19
>Reporter: Emmanuel Lecharny
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.1.1
>
>
> When we process incoming UDP messages, we iterate over the activated 
> {{SelectionKey}}s. That's ok, but for each one of them, we read the data and 
> try to flush the scheduled messages. The loop where it's done seems to 
> iterate on *all* the managed sessions, for each keys we are processing :
> {code:java}
>private void processReadySessions(Set handles) {
> Iterator iterator = handles.iterator();
> while (iterator.hasNext()) {
> SelectionKey key = iterator.next();
> DatagramChannel handle = (DatagramChannel) key.channel();
> iterator.remove();
> try {
> if (key.isValid() && key.isReadable()) {
> readHandle(handle);
> }
> if (key.isValid() && key.isWritable()) {
> for (IoSession session : getManagedSessions().values()) {
> scheduleFlush((NioSession) session);
> ...
> {code}
> There is no reason to do so. First, we should not iterate on all the managed 
> sessions (we may have thousands), but only the sessions which have something 
> to write, and we certainly should not do that for every active key...



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Created] (DIRMINA-1104) IoBufferHexDumper.getHexdump(IoBuffer in, int lengthLimit) does not truncate the output

2019-04-12 Thread Emmanuel Lecharny (JIRA)
Emmanuel Lecharny created DIRMINA-1104:
--

 Summary: IoBufferHexDumper.getHexdump(IoBuffer in, int 
lengthLimit) does not truncate the output
 Key: DIRMINA-1104
 URL: https://issues.apache.org/jira/browse/DIRMINA-1104
 Project: MINA
  Issue Type: Bug
Affects Versions: 2.1.0, 2.0.21
Reporter: Emmanuel Lecharny


When the {{lengthLimit}} is below the {{IoBuffer}} limit, the resulting dump 
should be truncated. It's not. Instead, "..." are appended at the end of the 
resulting String. I suspect that those "..." are supposed to be added when the 
result is *really* truncated.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-05 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16810763#comment-16810763
 ] 

Emmanuel Lecharny edited comment on DIRMINA-1103 at 4/5/19 12:36 PM:
-

Cool ! So I guess we can close this ticket and mark it as resolved?

Well, I guess we need to call the {{cleanup}} method at the end of {{inflate}} 
and {{deflate}}.


was (Author: elecharny):
Cool ! So I guess we can close this ticket and mark it as resolved?

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-05 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16810763#comment-16810763
 ] 

Emmanuel Lecharny commented on DIRMINA-1103:


Cool ! So I guess we can close this ticket and mark it as resolved?

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-05 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16810675#comment-16810675
 ] 

Emmanuel Lecharny commented on DIRMINA-1103:


Question: there is a {{cleanup}} method in {{zlib}} :

{code:java}
/**
 * Cleans up the resources used by the compression library.
 */
public void cleanUp() {
if (zStream != null) {
zStream.free();
}
}
{code}

Wouldn't it do the job ?

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16810356#comment-16810356
 ] 

Emmanuel Lecharny commented on DIRMINA-1103:


As Jonathan said, releasing the private buffer might leads to many creations of 
those buffers, if the session is used frequently. Reusing the existing buffers 
saves CPU and GC time.

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16810179#comment-16810179
 ] 

Emmanuel Lecharny commented on DIRMINA-1103:


Ok, makes sense. The idea would be to add a flag to this filter - 
{{resetBuffers}} - that can be used by the application. The default behaviour 
would remain what it currently is unless this flag is set.

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1103) Avoid sticky byte array buffers in Zlib CompressionFilter

2019-04-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1103?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16809906#comment-16809906
 ] 

Emmanuel Lecharny commented on DIRMINA-1103:


As Jonathan said on the mailing list, resetting the {{zlib}} buffers after each 
call would be done a the cost of creating them back when handling a new 
message.Now, there are various scenarii:
 * each session is only used once. That's ok, when the session is 'killed', the 
{{zlib}} instance get garbled
 * each session can send many messages. You may want to reuse the {{zlib}} 
instance with its existing buffers, instead of creating them again and again 
for each compressed/decompressed messages
 * you have many, many sessions, with only a few of them active at a given 
time. In this case, not holding the{{ }}{{zlib}} buffers forever makes sense, 
they are only useful for the sessions being processed. Otherwise, you may have 
a problem with all the pending buffers which are not used but eating the memory

I guess you are trying to get the third scenario implemented, right ?

> Avoid sticky byte array buffers in Zlib CompressionFilter
> -
>
> Key: DIRMINA-1103
> URL: https://issues.apache.org/jira/browse/DIRMINA-1103
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.21
>Reporter: Jörg Michelberger
>Priority: Major
>
> For each Zlib instance there is a zStream instance which is equipped with new 
> buffers for each in and out of inflate / deflate. This buffers are not 
> released after each inflate / deflate. So the last compressed and 
> uncompressed byte[] and its content for rx and tx is not garbage collectable 
> on each session.
> It would be great if there was a reset(zStream) method on Zlib which sets the 
> in and out to static empty byte[] to release these buffers after usage quite 
> before returning from Zlib.deflate and Zlib.inflate but in the synchronized 
> block. Something like:
> {{private void resetZStream (zStream) {}}
> {{  zStream.next_in = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_in_index = 0;}}
>  {{  zStream.avail_in = 0;}}
>  {{  zStream.next_out = EMPTY_BYTE_ARRAY;}}
>  {{  zStream.next_out_index = 0;}}
>  {{  zStream.avail_out = 0;}}
> {{}}}
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1102) FilterEvent must be exported by mina-core

2019-04-04 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1102.

Resolution: Fixed

Fixed with commit 3c51f9211cb4992e528655a46df4dfabf592332d.

A 2.1.1 will be released this week-end or next week.

> FilterEvent must be exported by mina-core
> -
>
> Key: DIRMINA-1102
> URL: https://issues.apache.org/jira/browse/DIRMINA-1102
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.1.0
>Reporter: Georgi Andreev
>Priority: Major
>
> I tried to use mina-core 2.1.0 in an OSGi environment, but this did not work 
> out of the box. There are a few extended interfaces like IoFilter which use 
> FilterEvent in the method signature. When this interface is extended the new 
> class needs to import org.apache.mina.filter package, but it is not exported 
> by mina-core. Please add the package to the Export-Package definition.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1102) FilterEvent must be exported by mina-core

2019-04-04 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16809599#comment-16809599
 ] 

Emmanuel Lecharny commented on DIRMINA-1102:


You are absolutely right ! Good catch.

> FilterEvent must be exported by mina-core
> -
>
> Key: DIRMINA-1102
> URL: https://issues.apache.org/jira/browse/DIRMINA-1102
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.1.0
>Reporter: Georgi Andreev
>Priority: Major
>
> I tried to use mina-core 2.1.0 in an OSGi environment, but this did not work 
> out of the box. There are a few extended interfaces like IoFilter which use 
> FilterEvent in the method signature. When this interface is extended the new 
> class needs to import org.apache.mina.filter package, but it is not exported 
> by mina-core. Please add the package to the Export-Package definition.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-04-03 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16808774#comment-16808774
 ] 

Emmanuel Lecharny commented on DIRMINA-1101:


As Jonathan said, the risk is that it breaks something in existing use code 
base, something I really can't guarantee with a modified 2.0.X.

OTOH, 2.1 is really close to 2.0, and there should be no impact on your code if 
you switch to this version. All in all, we just added an event, which is 
handled by default in the \{IoHandlerAdapter}, so your \{IoHandler} 
implementation should not be hit unless you were using the SSL notification 
(check that: [http://mina.apache.org/mina-project/2.1-vs-2.0.html).]

Is that the case ?

> InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)}}
>  {{    at java.lang.Thread.run(Thread.java:748)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Comment Edited] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-04-03 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16808774#comment-16808774
 ] 

Emmanuel Lecharny edited comment on DIRMINA-1101 at 4/3/19 2:23 PM:


As Jonathan said, the risk is that it breaks something in existing use code 
base, something I really can't guarantee with a modified 2.0.X.

OTOH, 2.1 is really close to 2.0, and there should be no impact on your code if 
you switch to this version. All in all, we just added an event, which is 
handled by default in the {{IoHandlerAdapter}}, so your {{IoHandler}} 
implementation should not be hit unless you were using the SSL notification 
(check that: [http://mina.apache.org/mina-project/2.1-vs-2.0.html).]

Is that the case ?


was (Author: elecharny):
As Jonathan said, the risk is that it breaks something in existing use code 
base, something I really can't guarantee with a modified 2.0.X.

OTOH, 2.1 is really close to 2.0, and there should be no impact on your code if 
you switch to this version. All in all, we just added an event, which is 
handled by default in the \{IoHandlerAdapter}, so your \{IoHandler} 
implementation should not be hit unless you were using the SSL notification 
(check that: [http://mina.apache.org/mina-project/2.1-vs-2.0.html).]

Is that the case ?

> InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> 

[jira] [Commented] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-03-26 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16801813#comment-16801813
 ] 

Emmanuel Lecharny commented on DIRMINA-1101:


There is definitively a plan to get a 2.1.1 release - and probably a 2.0.21 
release too - asap.

This is a serious bug.

> InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)}}
>  {{    at java.lang.Thread.run(Thread.java:748)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-03-26 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1101?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16801547#comment-16801547
 ] 

Emmanuel Lecharny commented on DIRMINA-1101:


Hi Guus,

this is a complex bug. The way we handle written message is, to say the least, 
convoluted. I w InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)}}
>  {{    at java.lang.Thread.run(Thread.java:748)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Assigned] (DIRMINA-1101) InvalidMarkException on session.write when using CompressionFilter.

2019-03-26 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny reassigned DIRMINA-1101:
--

Assignee: Emmanuel Lecharny

> InvalidMarkException on session.write when using CompressionFilter.
> ---
>
> Key: DIRMINA-1101
> URL: https://issues.apache.org/jira/browse/DIRMINA-1101
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.20
>Reporter: Jörg Michelberger
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> I'm updated from a MINA 2.0.7 to the 2.0.20 and am a user of 
> CompressionFilter. Writing of Messages fails with a InvalidMarkException.
> Reproducible Test is:
>  * Copy MINA Core Test org.apache.mina.core.service.AbstractIoServiceTest to 
> MINA Compression Filter org.apache.mina.filter.compression Test Packages 
> package.
>  * Adapt package statement to org.apache.mina.filter.compression.
>  * Add Compression to acceptor and connector
>  *     acceptor.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      acceptor.getFilterChain().addLast("logger", new LoggingFilter());
>      acceptor.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  *     connector.getFilterChain().addLast("compression", new 
> CompressionFilter());
>      connector.getFilterChain().addLast("logger", new LoggingFilter());
>      connector.getFilterChain().addLast("codec",
>      new ProtocolCodecFilter(new 
> TextLineCodecFactory(StandardCharsets.UTF_8)));
>  * Set a Breakpoint to java.nio.Buffer.reset() method, where the 
> InvalidMarkException is thrown.
>  * Run Debug Testfile on 
> org.apache.mina.filter.compression.AbstractIoServiceTest 
> After the Exception the session is immediatelly scheduled for disconnect.
> It seems that there is a discrepancy between the mark() and reset() calls on 
> the underlaying Buffer. In case of compression, a Buffer with the compressed 
> content is created and is wrapped with the original Buffer in a 
> FilteredWriteRequest, because CompressionFilter is a WriteRequestFilter. This 
> is in WriteRequestFilter.filterWrite()
> In DefaultIoFilterChain$HeadFilter.filterWrite() is then the mark() call, 
> which is done on the compressed Buffer.
> In AbstractPollingIoProcessor.writeBuffer() is the reset() call, which is in 
> this case done on the original Buffer, leading to the Exception.
> It seems that the change at date 16.02.2016
> SHA-1: 44b58469f84ce991074cdc187b1c1f23b94cf445
> * Don't try to reset a message when it's not a IoBuffer
> which reassignes the buf before reset() is called broke it. The buf before 
> reassign looks much better as the right to reset() in this case.
>  
> {{java.nio.InvalidMarkException}}
>  {{    at java.nio.Buffer.reset(Buffer.java:306)}}
>  {{    at 
> org.apache.mina.core.buffer.AbstractIoBuffer.reset(AbstractIoBuffer.java:425)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.writeBuffer(AbstractPollingIoProcessor.java:1131)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flushNow(AbstractPollingIoProcessor.java:994)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.flush(AbstractPollingIoProcessor.java:921)}}
>  {{    at 
> org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:688)}}
>  {{    at 
> org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)}}
>  {{    at 
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)}}
>  {{    at java.lang.Thread.run(Thread.java:748)}}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1095) Seems like the management f UDP sessions is really unneficient

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1095:
---
Fix Version/s: (was: 2.0.20)
   2.1.0

> Seems like the management f UDP sessions is really unneficient
> --
>
> Key: DIRMINA-1095
> URL: https://issues.apache.org/jira/browse/DIRMINA-1095
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.19
>Reporter: Emmanuel Lecharny
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.1.0
>
>
> When we process incoming UDP messages, we iterate over the activated 
> {{SelectionKey}}s. That's ok, but for each one of them, we read the data and 
> try to flush the scheduled messages. The loop where it's done seems to 
> iterate on *all* the managed sessions, for each keys we are processing :
> {code:java}
>private void processReadySessions(Set handles) {
> Iterator iterator = handles.iterator();
> while (iterator.hasNext()) {
> SelectionKey key = iterator.next();
> DatagramChannel handle = (DatagramChannel) key.channel();
> iterator.remove();
> try {
> if (key.isValid() && key.isReadable()) {
> readHandle(handle);
> }
> if (key.isValid() && key.isWritable()) {
> for (IoSession session : getManagedSessions().values()) {
> scheduleFlush((NioSession) session);
> ...
> {code}
> There is no reason to do so. First, we should not iterate on all the managed 
> sessions (we may have thousands), but only the sessions which have something 
> to write, and we certainly should not do that for every active key...



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1062) Sometimes ProtocolCodecFilter is not invoked on messageReceived processing

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1062.

Resolution: Incomplete

2

> Sometimes ProtocolCodecFilter is not invoked on messageReceived processing
> --
>
> Key: DIRMINA-1062
> URL: https://issues.apache.org/jira/browse/DIRMINA-1062
> Project: MINA
>  Issue Type: Bug
>Affects Versions: 2.0.16
>Reporter: Oleksii Osypov
>Priority: Major
>
> Clean connection to service on same host via external IP (not loopback) with 
> 3 filters in the chain:
> DefaultIoFilterChainBuilder chain = connector.getFilterChain();
> chain.addFirst("sslFilter", sslFilter);
> chain.addLast("codec", new ProtocolCodecFilter(new 
> RemoteAccessProtocolFactory()));
> chain.addLast("logger", new LoggingFilter())
> On first message write, instead of decoded protocol message object we receive 
> buffer object with "seems to be correct" binary data.
> Session "cc330198-bf1a-49a4-84aa-b5da2e49621e-3" invalid message: 
> HeapBuffer[pos=0 lim=43 cap=103: 2A 0A 26 63 63 33 33 30 31 39 38 2D 62 66 31 
> 61...]
> Stack:
> controller.remote.ExtendedIoHandler.messageReceived(ExtendedIoHandler.java:138)
>  ~[controller-common-1.9.0.jar:1.9.0]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$TailFilter.messageReceived(DefaultIoFilterChain.java:858)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:542)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:48)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:947)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.filter.logging.LoggingFilter.messageReceived(LoggingFilter.java:208)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:542)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:48)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:947)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.filter.ssl.SslHandler.flushScheduledEvents(SslHandler.java:326)
>  ~[mina-core-2.0.16.jar:?]
>   at org.apache.mina.filter.ssl.SslFilter.filterWrite(SslFilter.java:653) 
> ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callPreviousFilterWrite(DefaultIoFilterChain.java:629)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1500(DefaultIoFilterChain.java:48)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.filterWrite(DefaultIoFilterChain.java:957)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.filter.codec.ProtocolCodecFilter.filterWrite(ProtocolCodecFilter.java:317)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callPreviousFilterWrite(DefaultIoFilterChain.java:629)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1500(DefaultIoFilterChain.java:48)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.filterWrite(DefaultIoFilterChain.java:957)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.IoFilterAdapter.filterWrite(IoFilterAdapter.java:123)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callPreviousFilterWrite(DefaultIoFilterChain.java:629)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1500(DefaultIoFilterChain.java:48)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.filterWrite(DefaultIoFilterChain.java:957)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain$TailFilter.filterWrite(DefaultIoFilterChain.java:881)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.callPreviousFilterWrite(DefaultIoFilterChain.java:629)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.filterchain.DefaultIoFilterChain.fireFilterWrite(DefaultIoFilterChain.java:622)
>  ~[mina-core-2.0.16.jar:?]
>   at 
> org.apache.mina.core.session.AbstractIoSession.write(AbstractIoSession.java:574)
>  

[jira] [Updated] (DIRMINA-1079) MINA fails to connect from behind a proxy if endpoint is not resolved

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1079:
---
Fix Version/s: 2.0.21

> MINA fails to connect from behind a proxy if endpoint is not resolved
> -
>
> Key: DIRMINA-1079
> URL: https://issues.apache.org/jira/browse/DIRMINA-1079
> Project: MINA
>  Issue Type: Bug
>  Components: Handler
>Affects Versions: 2.0.16
>Reporter: Anton Novikov
>Priority: Major
> Fix For: 2.0.21
>
> Attachments: DIRMINA-1079.patch
>
>
> MINA fails to connect from behind a proxy if endpoint address is not 
> resolved. This happens for both HTTP and SOCKS proxy and it seems that the 
> reason for this are typos in HttpProxyRequest and SocksProxyRequest. The 
> following changes seem to fix the issue:
> HttpProxyRequest line 105: replace {{if (!endpointAddress.isUnresolved()) {}} 
> with {{if (endpointAddress.isUnresolved()) {}}
> SocksProxyRequest line 162: replace {{if (adr != null && !adr.isUnresolved()) 
> {}} with {{if (adr != null && adr.isUnresolved()) {}}
> Note the negation is gone in both cases



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1092) Annoying printStackTrace

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1092.

Resolution: Fixed

Fixed

> Annoying printStackTrace
> 
>
> Key: DIRMINA-1092
> URL: https://issues.apache.org/jira/browse/DIRMINA-1092
> Project: MINA
>  Issue Type: Bug
>  Components: Transport
>Affects Versions: 2.0.19
> Environment: JDK 10.0.2, mina-core 2.0.19, Debian 8
>Reporter: Hendi
>Priority: Trivial
> Fix For: 2.0.20
>
>
> Could you please remove the printStackTrace from 
> AbstractPollingIoProcessor$Processor.writeBuffer, around line 1109? It was 
> introduced in a recent version of MINA and spams the log with occasional 
> "Broken pipe" IOExceptions among other ones. Either remove it or pass the 
> exception up to exceptionCaught.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1093) Ignore SME just like CNFE while checking deadlock in DefaultIoFuture

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1093:
---
Fix Version/s: 2.0.21

> Ignore SME just like CNFE while checking deadlock in DefaultIoFuture
> 
>
> Key: DIRMINA-1093
> URL: https://issues.apache.org/jira/browse/DIRMINA-1093
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.17, 2.0.18, 2.0.19
> Environment: Applicable to all platforms
>Reporter: Yogesh Gaikwad
>Priority: Major
>  Labels: easyfix
> Fix For: 2.0.21
>
> Attachments: DIRMINA-1093.patch
>
>
> The problem occurs when we have a security manager enabled with restrictions.
> After writing a request to 
> [LdapNetworkConnection|https://github.com/apache/directory-ldap-api/blob/52129503d7f2247bad4a5f73b234023b8158ff07/ldap/client/api/src/main/java/org/apache/directory/ldap/client/api/LdapNetworkConnection.java#L4934],
>  we wait for the completion of the request. For every 
> {code:java}writeFuture.awaitUninterruptibly{code} it checks for deadlock 
> [DefaultIoFuture|https://mina.apache.org/mina-project/apidocs/src-html/org/apache/mina/core/future/DefaultIoFuture.html]#checkDeadLock()
>  which is code that requires security permissions and throws following 
> exception:
> {code:java}java.security.AccessControlException: access denied 
> ("java.lang.RuntimePermission" "accessClassInPackage.sun.reflect"){code}
> A proposed solution would be to ignore Java security manager exception (SME) 
> similar to Class not found exception(CNFE). For CNFE, we ignore the exception 
> and continue as we are just testing for deadlocks.
> {code:java}
> for (StackTraceElement s : stackTrace) {
>  try {
> Class cls = 
> DefaultIoFuture.class.getClassLoader().loadClass(s.getClassName());
> 
> if (IoProcessor.class.isAssignableFrom(cls)) {
> throw new IllegalStateException("DEAD LOCK: " + 
> IoFuture.class.getSimpleName()
> + ".await() was invoked from an I/O processor 
> thread.  " + "Please use "
> + IoFutureListener.class.getSimpleName()
> + " or configure a proper thread model 
> alternatively.");
> }
> } catch (ClassNotFoundException cnfe) {
> // Ignore
> }
> }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Updated] (DIRMINA-1092) Annoying printStackTrace

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny updated DIRMINA-1092:
---
Fix Version/s: 2.0.20

> Annoying printStackTrace
> 
>
> Key: DIRMINA-1092
> URL: https://issues.apache.org/jira/browse/DIRMINA-1092
> Project: MINA
>  Issue Type: Bug
>  Components: Transport
>Affects Versions: 2.0.19
> Environment: JDK 10.0.2, mina-core 2.0.19, Debian 8
>Reporter: Hendi
>Priority: Trivial
> Fix For: 2.0.20
>
>
> Could you please remove the printStackTrace from 
> AbstractPollingIoProcessor$Processor.writeBuffer, around line 1109? It was 
> introduced in a recent version of MINA and spams the log with occasional 
> "Broken pipe" IOExceptions among other ones. Either remove it or pass the 
> exception up to exceptionCaught.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1087) Backward incompatible change for SslFilter#SESSION_SECURED

2019-02-21 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1087.

   Resolution: Fixed
Fix Version/s: (was: 2.0.20)
   2.0.19

Has been fixed by 2.0.19

> Backward incompatible change for SslFilter#SESSION_SECURED
> --
>
> Key: DIRMINA-1087
> URL: https://issues.apache.org/jira/browse/DIRMINA-1087
> Project: MINA
>  Issue Type: Bug
>  Components: SSL
>Affects Versions: 2.0.18
>Reporter: Goldstein Lyor
>Priority: Major
>  Labels: backward-incompatible
> Fix For: 2.0.19
>
>
> Since {{SESSION_SECURED}} and {{SESSION_UNSECURED}} have been moved from 
> {{SslFilter}} to {{Event}}, the Apache directory server class 
> {{LdapProtocolHandler#messageReceived}} can no longer function properly. Some 
> degree of backward compatibility should have been preserved (IMO) .



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1089) Review the download page links

2019-02-20 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772822#comment-16772822
 ] 

Emmanuel Lecharny commented on DIRMINA-1089:


* We should remove any reference on {{MD5}} : DONE

> Review the download page links
> --
>
> Key: DIRMINA-1089
> URL: https://issues.apache.org/jira/browse/DIRMINA-1089
> Project: MINA
>  Issue Type: Task
>Reporter: Emmanuel Lecharny
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> The download pages must respect those rules :
> * links on signatures must start with {{https://www.apache.org/dist}}
> * We should remove any reference on {{MD5}}
> * Define a hash using ASC, SHA1, SHA256 and SHA512 only
> * Check the links before sending the announcement
> That has to be checked for MINA, FTPSERVER, VYSPER and SSHD.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Resolved] (DIRMINA-1089) Review the download page links

2019-02-20 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny resolved DIRMINA-1089.

Resolution: Fixed

I think we are good to go.

We just have to make sure that the generated signatures are correct (ie, 
include only sha1/256/512/asc)

> Review the download page links
> --
>
> Key: DIRMINA-1089
> URL: https://issues.apache.org/jira/browse/DIRMINA-1089
> Project: MINA
>  Issue Type: Task
>Reporter: Emmanuel Lecharny
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> The download pages must respect those rules :
> * links on signatures must start with {{https://www.apache.org/dist}}
> * We should remove any reference on {{MD5}}
> * Define a hash using ASC, SHA1, SHA256 and SHA512 only
> * Check the links before sending the announcement
> That has to be checked for MINA, FTPSERVER, VYSPER and SSHD.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1089) Review the download page links

2019-02-20 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1089?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16772751#comment-16772751
 ] 

Emmanuel Lecharny commented on DIRMINA-1089:


* links on signatures must start with {{[https://www.apache.org/dist]}} : DONE

FTR, I replaced all the [http://www.apache.org|http://www.apache.org/] links to 
use https instead. All teh Apache site is available through https.

> Review the download page links
> --
>
> Key: DIRMINA-1089
> URL: https://issues.apache.org/jira/browse/DIRMINA-1089
> Project: MINA
>  Issue Type: Task
>Reporter: Emmanuel Lecharny
>Assignee: Emmanuel Lecharny
>Priority: Major
>
> The download pages must respect those rules :
> * links on signatures must start with {{https://www.apache.org/dist}}
> * We should remove any reference on {{MD5}}
> * Define a hash using ASC, SHA1, SHA256 and SHA512 only
> * Check the links before sending the announcement
> That has to be checked for MINA, FTPSERVER, VYSPER and SSHD.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1098) Hello, I have found a bug in 2.0.19

2019-02-01 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16758541#comment-16758541
 ] 

Emmanuel Lecharny commented on DIRMINA-1098:


Yes.

> Hello, I have  found a bug in 2.0.19
> 
>
> Key: DIRMINA-1098
> URL: https://issues.apache.org/jira/browse/DIRMINA-1098
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.19
>Reporter: cheng
>Priority: Blocker
> Fix For: 3.0.0-trunk
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> First of all, I want to say, I have found a bug in version 2.0.19, which is 
> no problem in version 2.0.16. Comparing the 
> org.apache.mina.filter.ssl.SslHandle.java betwen two version, in the unwrap() 
> method, handshakeStatus is a temporary variable in version 2.0.16, and this 
> temporary variable is changed to a member variable in 2.0.19.
> The problem is that when the session is established, if the server 
> immediately sends a message, the handshakeStatus will remain in the 
> NOT_HANDSHAKING state at this time, and will never go to FINISHED.
> {code:java}
> //2.0.19 has a bug
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> {code:java}
> //2.0.16 is ok
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> The following is my positioning process, please refer to.
> 1. In the handshake method of SslHandler.java, add a sentence to print.
> {code:java}
> //2.0.19
> /void handshake(NextFilter nextFilter) throws SSLException {
>     for (;;) {
>     switch (handshakeStatus) {
>     System.out.println(handshakeStatus);
>     case FINISHED:
> {code}
> When creating an ssl connection, the 2.0.19 print sequence is as follows: 
> NOT_HANDSHAKING
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> {color:#FF} NOT_HANDSHAKING{color}
> {color:#FF} NOT_HANDSHAKING {color}
>   And 2.0.16 can be ended correctly.
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> FINISHED
> 2. We can find thar there is a problem in the last NEED_UNWRAP. By debugging 
> the code in 2.0.16 and 2.0.19 respectively, When the unwrapHandshake method 
> is tracked , the handshakeStatus variable is changed after the unwrap() 
> method. However, when viewing the unwrap() method, there is no reassignment 
> of the variables to the handshakeStatus, so I guess the reference address was 
> changed. By printing the memory address of the handshakeStatus, it was found 
> that the change did occur. As for where to change, I didn't understand at the 
> moment, so I compared the modification points of the SslHandler.java files in 
>  2.0.16 and 2.0.19 , then found that the temporary variable in 2.0.19 was 
> wrongly changed to member variable ,maybe due to the name is same.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter)
>  throws SSLException {
>    ***
>  
>    // If handshake finished, no data was produced, and the status is 
> still
>    // ok, try to unwrap more
>    if ((handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED)
>   && (res.getStatus() == SSLEngineResult.Status.OK)
>   && inNetBuffer.hasRemaining()) {
>  res = unwrap();
> ***
> {code}
>  
> 3. Try to change handshakeStatus  to a temporary variable, I found that the 
> problem was fixed.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }
> {code}
>  
> Therefore, please help to track down the problem and solve it as soon as 
> possible. This problem is conditional appearance, thank you.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Moved] (SSHD-886) unable to connect from AIX 7.2, contains workaround

2019-02-01 Thread Emmanuel Lecharny (JIRA)


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

Emmanuel Lecharny moved DIRMINA-1099 to SSHD-886:
-

Affects Version/s: (was: 2.0.19)
  Component/s: (was: SSL)
  Key: SSHD-886  (was: DIRMINA-1099)
  Project: MINA SSHD  (was: MINA)

> unable to connect from AIX 7.2, contains workaround
> ---
>
> Key: SSHD-886
> URL: https://issues.apache.org/jira/browse/SSHD-886
> Project: MINA SSHD
>  Issue Type: Bug
> Environment: AIX 7.2 openssh client attempting connection to 
> Bitbucket 5.16.0 containing Mina 2.0.19
>Reporter: Antony Suter
>Priority: Major
>
> There is a potential bug in Mina in handling a custom SSH packet type 106. 
> "Unassigned" according to 
> [https://www.iana.org/assignments/ssh-parameters/ssh-parameters.xhtml]
> My scenario is attempting to git clone from the ssh server inside Bitbucket 
> 5.16.0, which is Mina 2.0.19.
> On Linux if I set:
> {{export GIT_SSH_COMMAND="ssh -vvv"}}
> Then I get this log fragment from my git clone failing:
> {{debug1: Host '[bitbucketdev]:7999' is known and matches the RSA host key.}}
> {{debug1: Found key in /home/<...>/.ssh/known_hosts:1}}
> {{debug3: send packet: type 21}}
> {{debug2: set_newkeys: mode 1}}
> {{debug1: rekey after 4294967296 blocks}}
> {{debug1: SSH2_MSG_NEWKEYS sent}}
> {{debug1: expecting SSH2_MSG_NEWKEYS}}
> {{debug3: receive packet: type 21}}
> {{debug1: SSH2_MSG_NEWKEYS received}}
> {{debug2: set_newkeys: mode 0}}
> {{debug1: rekey after 4294967296 blocks}}
> {{debug2: key: /home/<...>/.ssh/id_rsa (200855b8)}}
> {{debug2: key: /home/<...>/.ssh/id_dsa (0)}}
> {{debug2: key: /home/<...>/.ssh/id_ecdsa (0)}}
> {{debug2: key: /home/<...>/.ssh/id_ed25519 (0)}}
> {{debug3: send packet: type 5}}
> {{debug3: receive packet: type 6}}
> {{debug2: service_accept: ssh-userauth}}
> {{debug1: SSH2_MSG_SERVICE_ACCEPT received}}
> {{debug3: send packet: type 50}}
> {{debug3: receive packet: type 51}}
> {{debug1: Authentications that can continue: publickey}}
> {{debug3: start over, passed a different list publickey}}
> {{debug3: preferred publickey,keyboard-interactive,password}}
> {{debug3: authmethod_lookup publickey}}
> {{debug3: remaining preferred: keyboard-interactive,password}}
> {{debug3: authmethod_is_enabled publickey}}
> {{debug1: Next authentication method: publickey}}
> {{debug1: Offering RSA public key: /home/<...>/.ssh/id_rsa}}
> {{debug3: send_pubkey_test}}
> {{debug3: send packet: type 50}}
> {{debug2: we sent a publickey packet, wait for reply}}
> {{debug3: receive packet: type 60}}
> {{debug1: Server accepts key: pkalg ssh-rsa blen 279}}
> {{debug2: input_userauth_pk_ok: fp SHA256:<...>}}
> {{debug3: sign_and_send_pubkey: RSA SHA256:<...>}}
> {{debug3: send packet: type {color:#FF}106{color}}}
> {{debug1: Sent ALLOW_PKCS12_KEYSTORE_CLIENT_FLAG packet}}
> {{debug3: send packet: type 50}}
> {{debug3: receive packet: type 51}}
> {{debug1: Authentications that can continue: publickey}}
> {{debug1: Trying private key: /home/<...>/.ssh/id_dsa}}
> {{debug3: no such identity: /home/<...>/.ssh/id_dsa: No such file or 
> directory}}
> {{debug1: Trying private key: /home/<...>/.ssh/id_ecdsa}}
> {{debug3: no such identity: /home/<...>/.ssh/id_ecdsa: No such file or 
> directory}}
> {{debug1: Trying private key: /home/<...>/.ssh/id_ed25519}}
> {{debug3: no such identity: /home/<...>/.ssh/id_ed25519: No such file or 
> directory}}
> {{debug2: we did not send a packet, disable method}}
> {{debug1: No more authentication methods to try.}}
> {{Permission denied (publickey).}}
> {{fatal: Could not read from remote repository.}}{{Please make sure you have 
> the correct access rights}}
> {{and the repository exists.}}
> After my key is sent to Mina and accepted with Server accepts key (and 
> Bitbucket logs the acceptance), the AIX openssh client sends packet type 106, 
> then the key is rejected.
> The workaround is to set an option in my ~/.ssh/config file:
> {{AllowPKCS12keystoreAutoOpen no}}
> Then I can git clone successfully.
> This ssh option is custom and unrecognized on Linux openssh client.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1098) Hello, I have found a bug in 2.0.19

2019-01-20 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16747408#comment-16747408
 ] 

Emmanuel Lecharny commented on DIRMINA-1098:


Seems like it was fixed 6 months ago :
https://gitbox.apache.org/repos/asf?p=mina.git;a=commit;h=a01bb9a2af133c39241aca32d93ca433a76038a4

Except that we didn't cut a 2.0.20 release...

Ok, will work on that ASAP.

> Hello, I have  found a bug in 2.0.19
> 
>
> Key: DIRMINA-1098
> URL: https://issues.apache.org/jira/browse/DIRMINA-1098
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.19
>Reporter: cheng
>Priority: Blocker
> Fix For: 3.0.0-trunk
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> First of all, I want to say, I have found a bug in version 2.0.19, which is 
> no problem in version 2.0.16. Comparing the 
> org.apache.mina.filter.ssl.SslHandle.java betwen two version, in the unwrap() 
> method, handshakeStatus is a temporary variable in version 2.0.16, and this 
> temporary variable is changed to a member variable in 2.0.19.
> The problem is that when the session is established, if the server 
> immediately sends a message, the handshakeStatus will remain in the 
> NOT_HANDSHAKING state at this time, and will never go to FINISHED.
> {code:java}
> //2.0.19 has a bug
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> {code:java}
> //2.0.16 is ok
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> The following is my positioning process, please refer to.
> 1. In the handshake method of SslHandler.java, add a sentence to print.
> {code:java}
> //2.0.19
> /void handshake(NextFilter nextFilter) throws SSLException {
>     for (;;) {
>     switch (handshakeStatus) {
>     System.out.println(handshakeStatus);
>     case FINISHED:
> {code}
> When creating an ssl connection, the 2.0.19 print sequence is as follows: 
> NOT_HANDSHAKING
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> {color:#FF} NOT_HANDSHAKING{color}
> {color:#FF} NOT_HANDSHAKING {color}
>   And 2.0.16 can be ended correctly.
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> FINISHED
> 2. We can find thar there is a problem in the last NEED_UNWRAP. By debugging 
> the code in 2.0.16 and 2.0.19 respectively, When the unwrapHandshake method 
> is tracked , the handshakeStatus variable is changed after the unwrap() 
> method. However, when viewing the unwrap() method, there is no reassignment 
> of the variables to the handshakeStatus, so I guess the reference address was 
> changed. By printing the memory address of the handshakeStatus, it was found 
> that the change did occur. As for where to change, I didn't understand at the 
> moment, so I compared the modification points of the SslHandler.java files in 
>  2.0.16 and 2.0.19 , then found that the temporary variable in 2.0.19 was 
> wrongly changed to member variable ,maybe due to the name is same.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter)
>  throws SSLException {
>    ***
>  
>    // If handshake finished, no data was produced, and the status is 
> still
>    // ok, try to unwrap more
>    if ((handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED)
>   && (res.getStatus() == SSLEngineResult.Status.OK)
>   && inNetBuffer.hasRemaining()) {
>  res = unwrap();
> ***
> {code}
>  
> 3. Try to change handshakeStatus  to a temporary variable, I found that the 
> problem was fixed.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }
> {code}
>  
> Therefore, please help to track down the problem and solve it as soon as 
> possible. This problem is conditional appearance, thank you.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1098) Hello, I have found a bug in 2.0.19

2019-01-18 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1098?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16746683#comment-16746683
 ] 

Emmanuel Lecharny commented on DIRMINA-1098:


Thanks for the report, Cheng.

I'll have a closer look asap (and sorry for the delayed response)

> Hello, I have  found a bug in 2.0.19
> 
>
> Key: DIRMINA-1098
> URL: https://issues.apache.org/jira/browse/DIRMINA-1098
> Project: MINA
>  Issue Type: Bug
>  Components: Core
>Affects Versions: 2.0.19
>Reporter: cheng
>Priority: Blocker
> Fix For: 3.0.0-trunk
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> First of all, I want to say, I have found a bug in version 2.0.19, which is 
> no problem in version 2.0.16. Comparing the 
> org.apache.mina.filter.ssl.SslHandle.java betwen two version, in the unwrap() 
> method, handshakeStatus is a temporary variable in version 2.0.16, and this 
> temporary variable is changed to a member variable in 2.0.19.
> The problem is that when the session is established, if the server 
> immediately sends a message, the handshakeStatus will remain in the 
> NOT_HANDSHAKING state at this time, and will never go to FINISHED.
> {code:java}
> //2.0.19 has a bug
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> {code:java}
> //2.0.16 is ok
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }{code}
> The following is my positioning process, please refer to.
> 1. In the handshake method of SslHandler.java, add a sentence to print.
> {code:java}
> //2.0.19
> /void handshake(NextFilter nextFilter) throws SSLException {
>     for (;;) {
>     switch (handshakeStatus) {
>     System.out.println(handshakeStatus);
>     case FINISHED:
> {code}
> When creating an ssl connection, the 2.0.19 print sequence is as follows: 
> NOT_HANDSHAKING
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> {color:#FF} NOT_HANDSHAKING{color}
> {color:#FF} NOT_HANDSHAKING {color}
>   And 2.0.16 can be ended correctly.
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_TASK
> NEED_WRAP
> NEED_WRAP
> NEED_WRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> NEED_UNWRAP
> FINISHED
> 2. We can find thar there is a problem in the last NEED_UNWRAP. By debugging 
> the code in 2.0.16 and 2.0.19 respectively, When the unwrapHandshake method 
> is tracked , the handshakeStatus variable is changed after the unwrap() 
> method. However, when viewing the unwrap() method, there is no reassignment 
> of the variables to the handshakeStatus, so I guess the reference address was 
> changed. By printing the memory address of the handshakeStatus, it was found 
> that the change did occur. As for where to change, I didn't understand at the 
> moment, so I compared the modification points of the SslHandler.java files in 
>  2.0.16 and 2.0.19 , then found that the temporary variable in 2.0.19 was 
> wrongly changed to member variable ,maybe due to the name is same.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult.Status unwrapHandshake(NextFilter nextFilter)
>  throws SSLException {
>    ***
>  
>    // If handshake finished, no data was produced, and the status is 
> still
>    // ok, try to unwrap more
>    if ((handshakeStatus == SSLEngineResult.HandshakeStatus.FINISHED)
>   && (res.getStatus() == SSLEngineResult.Status.OK)
>   && inNetBuffer.hasRemaining()) {
>  res = unwrap();
> ***
> {code}
>  
> 3. Try to change handshakeStatus  to a temporary variable, I found that the 
> problem was fixed.
>  
> {code:java}
> //2.0.19
> private SSLEngineResult unwrap() throws SSLException {
> ***
> SSLEngineResult res;
> Status status;
> HandshakeStatus handshakeStatus = null;
> do {
> ***
> handshakeStatus = res.getHandshakeStatus();
> ***
> }
> {code}
>  
> Therefore, please help to track down the problem and solve it as soon as 
> possible. This problem is conditional appearance, thank you.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (DIRMINA-1095) Seems like the management f UDP sessions is really unneficient

2019-01-18 Thread Emmanuel Lecharny (JIRA)


[ 
https://issues.apache.org/jira/browse/DIRMINA-1095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16746680#comment-16746680
 ] 

Emmanuel Lecharny commented on DIRMINA-1095:


Jonathan,

sorry for the delayed response, way to many stuff on my plate those last 
weeks...

I think we should move to 2.1.x for this change. 2.0.x should probably be left 
for pure maintenance.

> Seems like the management f UDP sessions is really unneficient
> --
>
> Key: DIRMINA-1095
> URL: https://issues.apache.org/jira/browse/DIRMINA-1095
> Project: MINA
>  Issue Type: Improvement
>Affects Versions: 2.0.19
>Reporter: Emmanuel Lecharny
>Assignee: Jonathan Valliere
>Priority: Major
> Fix For: 2.0.20
>
>
> When we process incoming UDP messages, we iterate over the activated 
> {{SelectionKey}}s. That's ok, but for each one of them, we read the data and 
> try to flush the scheduled messages. The loop where it's done seems to 
> iterate on *all* the managed sessions, for each keys we are processing :
> {code:java}
>private void processReadySessions(Set handles) {
> Iterator iterator = handles.iterator();
> while (iterator.hasNext()) {
> SelectionKey key = iterator.next();
> DatagramChannel handle = (DatagramChannel) key.channel();
> iterator.remove();
> try {
> if (key.isValid() && key.isReadable()) {
> readHandle(handle);
> }
> if (key.isValid() && key.isWritable()) {
> for (IoSession session : getManagedSessions().values()) {
> scheduleFlush((NioSession) session);
> ...
> {code}
> There is no reason to do so. First, we should not iterate on all the managed 
> sessions (we may have thousands), but only the sessions which have something 
> to write, and we certainly should not do that for every active key...



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


  1   2   3   4   5   6   7   8   9   10   >