[BUG] JSQLParser Version : 5.1: CCJSqlParserUtil.parse timeout Not thread-safe #2221

Activity

1. relufi commented on Apr 8, 2025

   

   relufi

   on Apr 8, 2025

   Author

   More actions

   It is cross-thread

   parser.interrupted = true;

   It is possible for ! interrupted to be inlined into a register, resulting in interrupted always being false
2. relufi commented on Apr 8, 2025

   

   relufi

   on Apr 8, 2025

   Author

   More actions

   It is also unwise to use an ExecutorService to implement timeouts, forcing the parsing work to be limited by the number of threads in the thread pool

   This is an ideal way of implementation, only need to use a global thread ScheduledThreadPoolExecutor whether to mark a timeout, which will help the parse SQL is limited by the ExecutorService number of threads

   public static ScheduledThreadPoolExecutor scheduledThreadPoolExecutor = new ScheduledThreadPoolExecutor(1);
       private static final VarHandle IVH;
       static {
           try {
               IVH = MethodHandles.lookup().findVarHandle(CCJSqlParser.class, "interrupted", boolean.class);
           } catch (NoSuchFieldException | IllegalAccessException e) {
               throw new RuntimeException(e);
           }
       }
       public static Statement parseWithTimeOut(CCJSqlParser parser) throws ParseException {
           ScheduledFuture<?> schedule = scheduledThreadPoolExecutor.schedule(
                   () ->{
                       //todo or use AtomicBoolean.setOpaque  or use volatile  But there is an additional overhead
                       //todo It's not actually valid  Because CCJSqlParser gets interrupted as a normal variable
                       IVH.setOpaque(parser, true)
                   },
                   4000, TimeUnit.MILLISECONDS
           );
           try {
               return parser.Statement();
           } finally {
               schedule.cancel(false);
           }
       }
3. relufi commented on Apr 8, 2025

   

   relufi

   on Apr 8, 2025

   Author

   More actions

   Or just add CCJSqlParser! interrupted is replaced with a check to see if the current time is greater than the timeout, which allows us to disconnect from the thread altogether
4. manticore-projects commented on Apr 8, 2025

   

   manticore-projects

   on Apr 8, 2025

   Contributor

   More actions

   Greetings! Thank you very much for you input which I appreciate a lot. Although your comments are a bit scarce to help my understanding.

   1. the parser is not threadsafe by design (although I agree that an AtomicBoolean won't hurt)
   2. you methods look a lot like JDK17+ (while we are yet on JDK11)
   3. PRs are welcome of course!
5. JSQLParser

   locked and limited conversation to collaborators on Apr 8, 2025
6. manticore-projects

   converted this issue into a discussion #2222 on Apr 8, 2025