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

Database file not released after modifying structure #1691

Closed
chrisjlocke opened this Issue Dec 27, 2018 · 9 comments

Comments

Projects
None yet
4 participants
@chrisjlocke
Copy link
Contributor

chrisjlocke commented Dec 27, 2018

Details for the issue

This might be a Windows thing, so ideally, if someone can confirm on another OS? I presume as MacOS is based on Linux, it won't affect that or the specific Linux builds - @justinclift recently taught me about file locks on Linux ... or the lack thereof... 😉

If a database structure is modified (and even if it is then cancelled) and the database closed in DB4S, the file cannot be renamed/moved, as DB4S still has a lock open on it. Even if another database is opened (and subsequently closed) the original database is still held open.

image

Mainly logging in case its an indication of a file handling issue which could grow over time as DB4S is being used.

What did you do?

Open database. Go to 'Database Structure' tab. Modify a table. Add a new field. Cancel out of all dialogs. Close database. Try and rename file.

What did you expect to see?

The file renamed.

What did you see instead?

An error that DB4S still had the file open.

Useful extra information

Here is a screencast.
https://screencast-o-matic.com/watch/cFltQprNT6

0:10 - The file is open in DB4S, and the renaming fails as expected.
0:17 - The file is closed in DB4S and the renaming succeeds as expected.
0:30 - The file is opened in DB4S and the table modified.
0:37 - A field is added, but cancelled. The database is then closed
0:50 - The file cannot be renamed
0:55 - Another database is opened in DB4S and closed. This can be renamed, but the original file is still locked and cannot be renamed.
1:16 - DB4S is closed, and the original file can now be renamed.

The info below often helps, please fill it out if you're able to. :)

What operating system are you using?

  • Windows: ( version: 10_ )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

What is your DB4S version?

  • Nightly from 24 Dec 2018
  • 3.11.0-alpha1 or 3.11.0-beta*
  • 3.10.1
  • Other: ___

Did you also

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Dec 27, 2018

Does the same happen for you with a very small database? I'm asking because the only explanation I can come up with right now is that some background thread is still running and accessing the file.

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Dec 27, 2018

The file I used in the above test was pretty large (1.2 GB) but the file I experienced it on last night was an empty database (8192 bytes).

image

I'll try leaving it for a while and see if it unlocks after a 'timeout' period.

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Dec 27, 2018

Looking at what files are open and by who, could it be a Qt thing?

image

MKleusberg added a commit that referenced this issue Dec 27, 2018

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Dec 27, 2018

Turns out it was a plain and simple error in our code 😉 In this function (which is called when editing an existing table, besides some other places) you can see that the stmt variable is allocated by the sqlite3_prepare_v2 call. But we only call sqlite3_finalize on it if we got any results from sqlite3_step which we didn't in this case (which is probably a separate issue we should look into). And when there is even a single unfinalized statement, SQLite doesn't actually close the database - DB4S just pretends it did 😉

Anyway, this should be fixed in tomorrow's nightly build 😄

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Dec 27, 2018

Excellent, thanks! 🎆

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Dec 28, 2018

Can confirm this is now resolved. Thanks @MKleusberg for the prompt fix.

@mgrojo

This comment has been minimized.

Copy link
Contributor

mgrojo commented Dec 28, 2018

Maybe this is safe and simple enough to be added to v3.11.x.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Dec 28, 2018

If the bug is in 3.11.x, then we should definitely fix it there too. 😄

MKleusberg added a commit that referenced this issue Dec 28, 2018

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Dec 28, 2018

Just had the same thought 😉 It's now in the 3.11.x branch too.

mgrojo added a commit that referenced this issue Jan 3, 2019

Merge updated Spanish translation for v3.11.x (Fix possible resource …
…leaks)

This is an update of the translation file followed by a manual merge of
0ab9f44

See issue #1691
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment