deno-sqlite icon indicating copy to clipboard operation
deno-sqlite copied to clipboard

Broken locking logic

Open fabiospampinato opened this issue 2 years ago • 2 comments

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 avatar Aug 22 '23 01:08 fabiospampinato

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.

fabiospampinato avatar Aug 22 '23 11:08 fabiospampinato

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=)

dyedgreen avatar Aug 25 '23 11:08 dyedgreen