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

        Anonymous created issue -
        Show
        Ulrich Stärk added a comment - http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4729342
        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?
        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 -

        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 -

        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 -

        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
        Roger Schildmeijer made changes -
        Field Original Value New Value
        Priority Major [ 3 ]
        Roger Schildmeijer made changes -
        Fix Version/s 0.4.0 [ 12317348 ]
        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.
        Johnathan Meehan made changes -
        Assignee Johnathan Meehan [ jmeehan ]
        Johnathan Meehan made changes -
        Status Open [ 1 ] In Progress [ 3 ]
        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 -

        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 -

        I agree

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

        Ready for review.

        Show
        Johnathan Meehan added a comment - Ready for review.
        Johnathan Meehan made changes -
        Status In Progress [ 3 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Tony Stevenson made changes -
        Project Deft [ 12311521 ] Apache AWF [ 12313220 ]
        Key DEFT-149 AWF-16
        Reporter Niklas Gustavsson [ niklas ]
        Fix Version/s 0.4.0 [ 12317348 ]
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        57d 6h 5m 1 Johnathan Meehan 06/Sep/11 15:30
        In Progress In Progress Resolved Resolved
        24m 34s 1 Johnathan Meehan 06/Sep/11 15:55

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development