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

Broken locking logic #249

Open
fabiospampinato opened this issue Aug 22, 2023 · 2 comments
Open

Broken locking logic #249

fabiospampinato opened this issue Aug 22, 2023 · 2 comments
Labels
bug Something isn't working question Further information is requested

Comments

@fabiospampinato
Copy link

I'm not sure where the bug is exactly, but executing this code at the root of the project:

import {DB} from './mod.ts';

const db1 = new DB('foo.db');

db1.query ( `SELECT page_count * page_size as size FROM pragma_page_count(), pragma_page_size()` );

db1.close ();

And logging each time js_lock and js_unlock are called with something like this:

js_lock: (rid, exclusive) => {
  console.log ( `[js_lock] rid: ${rid}, exclusive: ${exclusive}` );
},
js_unlock: (rid) => {
  console.log ( `[js_unlock] rid: ${rid}` );
},

I get this log:

~/Downloads/deno-sqlite-master 2 ❯ deno run -A test.ts
[js_lock] rid: 3, exclusive: 0
[js_lock] rid: 3, exclusive: 0
~/Downloads/deno-sqlite-master 2 ❯ 

As we can see sqlite is requesting shared locks, but it's never releasing them. As long as there are shared locks an exclusive lock shouldn't be obtainable, so really sqlite here should be asking to unlock the database after its done with it, not asking for a shared lock twice, or it will prevent anybody else from writing to it, which can't be right.

I'm not sure what causes this bug. Maybe xCheckReservedLock can't be a sort of no-op?

This is worth investigating because without any form of locking that works I don't know how usable the approach really is.

@fabiospampinato
Copy link
Author

fabiospampinato commented Aug 22, 2023

Interestingly the equivalent code for node-sqlite3-wasm produces this output:

~/Desktop/sqlite3-wasm ❯ node test.js
[lock] rid 143528 - level 1
[unlock] rid 143528 - level 0
[lock] rid 143528 - level 1
[unlock] rid 143528 - level 0
[unlock] rid 143528 - level 0
[unlock] rid 143528 - level 0
~/Desktop/sqlite3-wasm ❯ 

So worth checking what they are doing differently, since sqlite seems to be making the right calls there.

They seem to be implementing xCheckReservedLock, I'm not sure if that's it or if there's more to the story.

@dyedgreen dyedgreen added the bug Something isn't working label Aug 25, 2023
@dyedgreen
Copy link
Owner

Yes, my guess would be that we have to implement that method. It's worth noting that the locking only works with --unstable, since Deno only provides an unstable locking API (see https://deno.land/[email protected]?s=Deno.flockSync&unstable=)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants