Menu ▾ ▴

#1690 JavaScriptJobManagerImpl waitForJobs and waitForJobsStartingBefore swallow InterruptedException

Latest SVN
accepted
nobody
None
1
2019-12-04
2015-06-04
Marios Chatzidimitriou
No

waitForJobs and waitForJobsStartingBefore methods in JavaScriptJobManagerImpl that are called from webClient.waitForBackgroundJavaScript and waitForBackgroundJavaScriptStartingBefore swallow InterruptedException. i.e. they catch it and do nothing, they just log:

 try {
         wait(end - now);
      }
      catch (final InterruptedException e) {
            LOG.error("InterruptedException while in waitForJobs", e);
      }

The effect of this is that applications that use HtmlUnit to parse web sites and have functionality that the user wishes to stop the parsing do not work properly if the user stops the parsing while the code is at the waitForJobs method. (I discovered this problem while working on an application that parses various sites to get prices, see http://stackoverflow.com/questions/28717215/htmlunit-webclient-resets-thread-interruption-status

The very helpful article at http://www.ibm.com/developerworks/java/library/j-jtp05236/index.html?ca=drs- explains that InterruptedException should never be swallowed, but it also provides the following workaround if it is not possible to rethrow it.

If you catch InterruptedException but cannot rethrow it, you should preserve evidence that the interruption occurred so that code higher up on the call stack can learn of the interruption and respond to it if it wants to. This task is accomplished by calling interrupt() to "reinterrupt" the current thread. At the very least, whenever you catch InterruptedException and don't rethrow it, reinterrupt the current thread before returning.

Therefore the proposed solutions are to either let the InterruptedException to be rethrown after logging it so that it propagates to the waitForJobs and waitForJobsStartingBefore method callers, or if this is not possible then you should reinterrupt the thread inside the catch method.
In that case code would become:

catch (final InterruptedException e) {
    LOG.error("InterruptedException while in waitForJobs", e);
    // Restore the interrupted status
    Thread.currentThread().interrupt();
}

Discussion

  • Ahmed Ashour

    Ahmed Ashour - 2015-06-05
    • status: open --> accepted
     

  • Ahmed Ashour

    Ahmed Ashour - 2015-06-05

    I tried to make a test case, but the test case fails most of times whenever we test with more than one browser.

    /**

 * Test for bug 1690.
 *
 * @throws Exception if the test fails
 */
@Test
public void interrupt() throws Exception {
    final String content = "<html>\n"
            + "<head>\n"
            + "  <title>test</title>\n"
            + "  <script>\n"
            + "    var threadID;\n"
            + "    function test() {\n"
            + "      threadID = setInterval(doAlert, 100);\n"
            + "    }\n"
            + "    function doAlert() {\n"
            + "      alert('blah');\n"
            + "    }\n"
            + "  </script>\n"
            + "</head>\n"
            + "<body onload='test()'>\n"
            + "</body>\n"
            + "</html>";

    final HtmlPage page = loadPage(content);
    final Thread mainThread = Thread.currentThread();

    final Thread thread = new Thread(new Runnable() {

        @Override
        public void run() {
            try {
                Thread.sleep(100);
            }
            catch(Exception e) {
            }
            mainThread.interrupt();
        }
    });
    thread.start();
    page.getWebClient().waitForBackgroundJavaScript(10000);
    assertTrue(mainThread.isInterrupted());
    thread.join();
    page.getWebClient().close();
}
     

  • Noman Alahi

    Noman Alahi - 2019-12-04

    Is there any progress on this issue ?

     

Log in to post a comment.

mdb logo