[jira] [Commented] (DIRMINA-1019) SslHandler flushScheduledEvents race condition

["Emmanuel Lecharny (JIRA)" <ji...@apache.org>]

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

Emmanuel Lecharny commented on DIRMINA-1019:
--------------------------------------------

Actually, I'm not even sure why we protected the second while loop with the sslLock. I have moved this loop out of the protected section, and teh test is passing :

{code}
    /* no qualifier */void flushScheduledEvents() {
        // Fire events only when the lock is available for this handler.
        IoFilterEvent event;
        try {
            sslLock.lock();

            // We need synchronization here inevitably because filterWrite can be
            // called simultaneously and cause 'bad record MAC' integrity error.
            while ((event = filterWriteEventQueue.poll()) != null) {
                NextFilter nextFilter = event.getNextFilter();
                nextFilter.filterWrite(session, (WriteRequest) event.getParameter());
            }
        } finally {
            sslLock.unlock();
        }

        while ((event = messageReceivedEventQueue.poll()) != null) {
            NextFilter nextFilter = event.getNextFilter();
            nextFilter.messageReceived(session, event.getParameter());
        }
{code}

> SslHandler flushScheduledEvents race condition
> ----------------------------------------------
>
>                 Key: DIRMINA-1019
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-1019
>             Project: MINA
>          Issue Type: Bug
>          Components: SSL
>    Affects Versions: 2.0.9
>            Reporter: Terence Marks
>             Fix For: 2.0.10
>
>         Attachments: SslFilterTest.java, fix.java
>
>
> From what I've seen, the SslFilter class schedules events onto the SslHandler and then flushes them via the SslHandler.flushScheduledEvents() method.
> Within SslHandler.flushScheduledEvents() the lock can only be accumulated by a single thread, and all other threads that try to accumulate the lock instead follow through and not block waiting to get the lock.
> ...
> if (sslLock.tryLock()) {
> ...
> This leads to the race condition where a thread may call SslHandler.scheduleFilterWrite() followed by SslHandler.flushScheduledEvents() while another thread holds the sslLock and has finished dequeuing the SslHandler.filterWriteEventQueue and is currently dequeuing the SslHandler.messageReceivedEventQueue.
> I've actually hit this race condition quite a few times today and have added in a small fix which essentially tracks the number of times SslHandler.flushScheduledEvents() has been called. There's probably a much better solution but yeah just letting you guys know what I've found.
> Thanks,
> Terence



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)