JSQLParser / JSqlParser

JSqlParser parses an SQL statement and translate it into a hierarchy of Java classes. The generated hierarchy can be navigated using the Visitor Pattern
https://github.com/JSQLParser/JSqlParser/wiki
Apache License 2.0
5.22k stars 1.33k forks source link

[BUG] JSQLParser 4.9 : RDBMS : ANY , parser swallows InterruptedException #2034

Closed relufi closed 1 week ago

relufi commented 2 weeks ago

com.github.jsqlparser jsqlparser 4.9

    public void testJsqlParser() throws JSQLParserException {
//        current thread object
        Thread thread = Thread.currentThread();
//        Set the interrupt flag of the current thread
        thread.interrupt();
        System.out.println("before: " + thread.isInterrupted());
        CCJSqlParserUtil.parseStatements("SELECT 1 FROM dual");
        System.out.println("after: " + thread.isInterrupted());
    }

current execution result

before: true after: false

expect:

before: true after: true or The parseStatements function throws InterruptedException

parseStatements swallows InterruptedException or interrupt flag This strange behavior caused me to not be able to properly cancel the executing task in the thread and close the thread pool

relufi commented 2 weeks ago

jsqlparser 5.0 also has this problem jsqlparser-5.0#CCJSqlParserUtil#L352

manticore-projects commented 2 weeks ago

Greetings.

Unfortunately I do not really understand what your complain is about. You can easily provide your own Exceutor and set timeouts. Why would you artificially interrupt the thread?

parseStatements throws a JSQLParseException because parsing failed and gives in the stack trace the reason of InterruptedException.

Until I hear a string argument or proposal for better implementation this will be a "Won't change".

relufi commented 2 weeks ago
        ExecutorService executorService = Executors.newFixedThreadPool(1);
        executorService.execute(() -> {
            try {
                while (true) {
                    CCJSqlParserUtil.parseStatements("SELECT 1 FROM dual");
                    if(Thread.interrupted()) {
//                        Unreachable because parseStatements changes the interrupt flag to false
                        System.out.println("interrupted task exit;");
                        return;
                    }
                }
            } catch (Exception e) {
//             Unreachable because parseStatements swallow InterruptedException
                e.printStackTrace();
            }
        });
//     Set the interrupt flag of the thread
        executorService.shutdownNow();
//     fail
        executorService.awaitTermination(1, TimeUnit.MINUTES);

parseStatements do not throw any exceptions and change the interrupt flag to false

relufi commented 2 weeks ago

It does not throw a JSQLParseException, In the internal, it will attempt to call the parseStatements function for a second time, but at this point, the interrupt flag has been reset to false. Therefore, the second execution of parseStatements will no longer throw an exception. However, this is not normal logic as it ignores being interrupted。

statements = parseStatements(parser.withAllowComplexParsing(true), executorService);

manticore-projects commented 2 weeks ago

ignores being interrupted。

Yes, because that's what the Timeout is for.

relufi commented 2 weeks ago
 public void testAsyncTask() throws Exception {
        ExecutorService executorService = Executors.newSingleThreadExecutor();
        Future<?> submit = executorService.submit(() -> {
            try {
    //            loop is used to simulate lengthy parse tasks
                for (int i = 0; i < 100; i++) {
                    CCJSqlParserUtil.parseStatements("SELECT 1 FROM dual");
                }
//                simulate execution SQL
                TimeUnit.MINUTES.sleep(10);

                System.out.println("end");
            }catch (Exception e) {
                e.printStackTrace();
            }
        });
        TimeUnit.MILLISECONDS.sleep(10);
        submit.cancel(true);
        executorService.awaitTermination(10, TimeUnit.MINUTES);
    }

If submit.cancel(true) occurs before or during the execution of the parseStatements, the sql execution cannot be canceled

manticore-projects commented 2 weeks ago

As I wrote before: You are NOT supposed to cancel the parsing, but rather to set the timeout after which the parser will fail.

I will not change anything here but you are welcome to provide a better implementation of course.