Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jsoup clearing thread interruption flag during I/O operations #1991

Closed
artemmukhin opened this issue Aug 14, 2023 · 2 comments
Closed

Jsoup clearing thread interruption flag during I/O operations #1991

artemmukhin opened this issue Aug 14, 2023 · 2 comments
Assignees
Labels
bug Confirmed bug that we should fix fixed
Milestone

Comments

@artemmukhin
Copy link

I've noticed an issue with how thread interruption handling is managed. I couldn't find any mention of this behaviour in the docs, and I'm not sure whether it's expected.

Jsoup appears to clear the interruption flag of the calling thread during I/O operations, which, I believe, leads to unexpected behaviour when trying to manage thread interruption.

Consider the following client code:

public void runWhileNotInterrupted() throws InterruptedException {
    final ArrayList<String> bodies = new ArrayList<>();
    Thread runner = new Thread(() -> {
        try {
            int i = 1;
            while (!Thread.currentThread().isInterrupted()) {
                Connection.Response res = Jsoup.connect("https://github.com/jhy/jsoup/issues/" + i).execute();
                bodies.add(res.body());
                i += 1;
            }
        } catch (IOException e) {
            throw new RuntimeException(e);
        }
    });

    runner.start();
    Thread.sleep(3 * 1000);
    runner.interrupt(); // Jsoup clears the flag
    runner.join(); // Thread never completes
    System.out.println(bodies);
}

The interruption flag is being cleared through java.lang.Thread#interrupted in ConstrainableInputStream.java:

public int read(byte[] b, int off, int len) throws IOException {
if (interrupted || capped && remaining <= 0)
return -1;
if (Thread.interrupted()) {
// interrupted latches, because parse() may call twice (and we still want the thread interupt to clear)
interrupted = true;
return -1;
}

I would expect either the interruption flag to be preserved or java.lang.InterruptedException to be thrown by Jsoup.

Please let me know if that behaviour is indeed unexpected. I will try to come up with a suitable solution and open a PR. Thanks!

@jhy jhy closed this as completed in 04b3a82 Sep 8, 2023
@jhy jhy self-assigned this Sep 8, 2023
@jhy jhy added bug Confirmed bug that we should fix fixed labels Sep 8, 2023
@jhy jhy added this to the 1.16.2 milestone Sep 8, 2023
@jhy
Copy link
Owner

jhy commented Sep 8, 2023

Thanks, good find! Fixed.

@artemmukhin
Copy link
Author

Thank you @jhy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug that we should fix fixed
Projects
None yet
Development

No branches or pull requests

2 participants