sqlitebrowser icon indicating copy to clipboard operation
sqlitebrowser copied to clipboard

[Bug]: `ROLLBACK;VACUUM;` indefinitely locks up application when switching tabs

Open AyrA opened this issue 1 year ago • 4 comments

What did you do?

  1. Create or open any database (file or memory)
  2. Execute ROLLBACK;VACUUM; in the sql editor
  3. Change the active tab

What did you expect to see?

The application should do either one of:

  • Change the tab and update contents when the query has completed
  • Inform the user that it's not possible to do so
  • Not executing SQL queries on the UI thread. Instead executing them in a time limited background thread providing graceful abort.

If we want to be very pedantic, the SQL statement should be aborted immediately, since there is no transaction opened by the user that ROLLBACK applies to, and it should not be possible to undermine the internal SQL state of the application.

What did you see instead?

The application indefinitely locks up. The only way out is to forcibly closing it, losing any and all unsaved SQL queries.

It seems that the ROLLBACK; terminates a transaction created by the application itself, leaving it in a corrupt state where it is unable to make any changes to the database whatsoever now. With the VACUUM command, the editor indefinitely waits for the internal transaction to complete but this is not possible since it has been rolled back. This is probably what then leads to it running indefinitely and eventually locking up the application because every command issued afterwards, whether by SQL editor or by an internal action gets queued forever.

DB4S Version

3.13.0

What OS are you seeing the problem on?

Windows

OS version

Version 10.0.19045.3208

Relevant log output

No response

Prevention against duplicate issues

  • [X] I have searched for similar issues

AyrA avatar Oct 14 '24 14:10 AyrA

This looks like a duplicate of #3723. Could you please confirm it is fixed with a continuous or nightly build?

mgrojo avatar Oct 14 '24 19:10 mgrojo

The nightly build no longer locks up, but there are still some issues with the same exact query:

  • Having VACUUM; as the last command (semicolon optional) returns an "Execution aborted by user" error unless you add an extra linebreak. For some reason only the first 4 letters of the command seem to be read. This seems to be specific to the vacuum command. If the last command is SELECT 1; it works without a trailing linebreak.
  • Running a ROLLBACK; without opening a transaction still doesn't throws an error. It just silently breaks the "Write Changes" button

AyrA avatar Oct 15 '24 02:10 AyrA

Cool, thanks for testing this @AyrA. Sounds like there's a bit more effort that still needs to go into it. :smile:

justinclift avatar Oct 15 '24 05:10 justinclift

@AyrA I fixed that corner case, could you confirm with the next nightly or continuous build that everything around this is working as expected?

If we want to be very pedantic, the SQL statement should be aborted immediately, since there is no transaction opened by the user that ROLLBACK applies to, and it should not be possible to undermine the internal SQL state of the application.

That's because DB4S runs the SQL statements in "Execute SQL" inside a transaction automatically opened by itself. That allows reverting the changes performed by the execution. You can see what DB4S does on its SQL Log.

mgrojo avatar Oct 19 '24 17:10 mgrojo

The ROLLBACK;VACCUM; command sequence seems to be working now.

In regards to the user rolling back a transaction they should not, you could check after every SQL command if you're in autocommit mode(see int sqlite3_get_autocommit(sqlite3*);). If you are, the user just rolled back a transaction they did not open and the script should be aborted, plus the transaction state of the application should be reset to a known state because the write and undo buttons stay broken otherwise and yell at you for not being able to execute a "release undopoint" command.

AyrA avatar Nov 06 '24 00:11 AyrA