Apache AWF
  1. Apache AWF
  2. AWF-16

IOLoop.start() may throw CancelledKeyException

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Labels:
      None

      Description

      Exception in thread "I/O-LOOP" java.nio.channels.CancelledKeyException
      at sun.nio.ch.SelectionKeyImpl.ensureValid(SelectionKeyImpl.java:55)
      at sun.nio.ch.SelectionKeyImpl.readyOps(SelectionKeyImpl.java:69)
      at java.nio.channels.SelectionKey.isAcceptable(SelectionKey.java:342)
      at org.deftserver.io.IOLoop.start(IOLoop.java:77)

      Would it be a reasonable solution to change the catch clause from catch(IOException e) to catch(Exception e)?

        Activity

        Hide
        Johnathan Meehan added a comment -

        Ready for review.

        Show
        Johnathan Meehan added a comment - Ready for review.
        Show
        Johnathan Meehan added a comment - Committed: http://svn.apache.org/viewvc?rev=1165691&view=rev
        Hide
        Roger Schildmeijer added a comment -

        I agree

        Show
        Roger Schildmeijer added a comment - I agree
        Hide
        Johnathan Meehan added a comment -

        Isn't it the case though that the server could continue to run without issue were this particular exception caught? In that case I think it would be better to catch, log and continue than allow it to fall over.

        Show
        Johnathan Meehan added a comment - Isn't it the case though that the server could continue to run without issue were this particular exception caught? In that case I think it would be better to catch, log and continue than allow it to fall over.
        Hide
        Roger Schildmeijer added a comment -

        Yes, sounds reasonable. I'm not a big fan of catch Exception/Throwable.

        Do you propose we close this issue as "won't fix"?

        Show
        Roger Schildmeijer added a comment - Yes, sounds reasonable. I'm not a big fan of catch Exception/Throwable. Do you propose we close this issue as "won't fix"?
        Hide
        Johnathan Meehan added a comment -

        Would it not actually be better to handle the exception known, rather than blanket cover any exception at all? I think the expected range of exceptions should be known, and the response to each explicit.

        Show
        Johnathan Meehan added a comment - Would it not actually be better to handle the exception known, rather than blanket cover any exception at all? I think the expected range of exceptions should be known, and the response to each explicit.
        Hide
        Ulrich Stärk added a comment -

        No problem, will do the diff manually. The best way (I guess) to do a proper diff is a) create a patch and paste/submit it here or on a pastebin or b) fork, commit, pull request.
        But thats not important. Thanks for the support by the way

        Show
        Ulrich Stärk added a comment - No problem, will do the diff manually. The best way (I guess) to do a proper diff is a) create a patch and paste/submit it here or on a pastebin or b) fork, commit, pull request. But thats not important. Thanks for the support by the way
        Hide
        Ulrich Stärk added a comment -

        well, not exactly, i've moved the try..catch clause around, but essentially, yes, it's just to catch the all the Exceptions so the web server won't exit when there's an unknown exception.

        Sorry I'm new to Github, not sure how to do the diff here

        Show
        Ulrich Stärk added a comment - well, not exactly, i've moved the try..catch clause around, but essentially, yes, it's just to catch the all the Exceptions so the web server won't exit when there's an unknown exception. Sorry I'm new to Github, not sure how to do the diff here
        Hide
        Ulrich Stärk added a comment -

        is the diff simply IOException => Exception ?

        Show
        Ulrich Stärk added a comment - is the diff simply IOException => Exception ?
        Hide
        Ulrich Stärk added a comment -

        it's in a single thread environment. i've changed the code to:

        /**

        • Start the io loop. The thread that invokes this method will be blocked
        • (until {@link IOLoop#stop}

          is invoked) and will be the io loop thread.
          */
          public void start() {
          Thread.currentThread().setName("I/O-LOOP");
          running = true;

        long selectorTimeout = 250; // 250 ms
        while (running) {
        try {
        if (selector.select(selectorTimeout) == 0) {
        long ms = tm.execute();
        selectorTimeout = Math.min(ms, /* selectorTimeout */250);
        if (cm.execute())

        { selectorTimeout = 1; }

        continue;
        }
        } catch (Exception e)

        { e.printStackTrace(); }

        Iterator<SelectionKey> keys = selector.selectedKeys().iterator();
        while (keys.hasNext()) {
        SelectionKey key = keys.next();
        IOHandler handler = handlers.get(key.channel());
        try {
        if (key.isAcceptable())

        { handler.handleAccept(key); }

        if (key.isConnectable())

        { handler.handleConnect(key); }

        if (key.isValid() && key.isReadable())

        { handler.handleRead(key); }

        if (key.isValid() && key.isWritable())

        { handler.handleWrite(key); }

        } catch (Exception e)

        { e.printStackTrace(); }

        keys.remove();
        }

        long ms = tm.execute();
        selectorTimeout = Math.min(ms, /* selectorTimeout */250);
        if (cm.execute())

        { selectorTimeout = 1; }

        }
        }

        Show
        Ulrich Stärk added a comment - it's in a single thread environment. i've changed the code to: /** Start the io loop. The thread that invokes this method will be blocked (until {@link IOLoop#stop} is invoked) and will be the io loop thread. */ public void start() { Thread.currentThread().setName("I/O-LOOP"); running = true; long selectorTimeout = 250; // 250 ms while (running) { try { if (selector.select(selectorTimeout) == 0) { long ms = tm.execute(); selectorTimeout = Math.min(ms, /* selectorTimeout */250); if (cm.execute()) { selectorTimeout = 1; } continue; } } catch (Exception e) { e.printStackTrace(); } Iterator<SelectionKey> keys = selector.selectedKeys().iterator(); while (keys.hasNext()) { SelectionKey key = keys.next(); IOHandler handler = handlers.get(key.channel()); try { if (key.isAcceptable()) { handler.handleAccept(key); } if (key.isConnectable()) { handler.handleConnect(key); } if (key.isValid() && key.isReadable()) { handler.handleRead(key); } if (key.isValid() && key.isWritable()) { handler.handleWrite(key); } } catch (Exception e) { e.printStackTrace(); } keys.remove(); } long ms = tm.execute(); selectorTimeout = Math.min(ms, /* selectorTimeout */250); if (cm.execute()) { selectorTimeout = 1; } } }
        Hide
        Ulrich Stärk added a comment -

        was this in a multithreaded environment? mind pasting some code?

        Show
        Ulrich Stärk added a comment - was this in a multithreaded environment? mind pasting some code?
        Show
        Ulrich Stärk added a comment - http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4729342

          People

          • Assignee:
            Johnathan Meehan
            Reporter:
            Niklas Gustavsson
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development