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

Calling .close() closes all existing DB Connections? #146

Closed
On2it-Clifford opened this issue Sep 18, 2024 · 18 comments
Closed

Calling .close() closes all existing DB Connections? #146

On2it-Clifford opened this issue Sep 18, 2024 · 18 comments
Assignees

Comments

@On2it-Clifford
Copy link

On2it-Clifford commented Sep 18, 2024

Describe the bug

let db1 = open({ name: 'test' });
let db2 = open({ name: 'test' });

db1.close();
db2.execute('Anything');

db2 will throw 'Error: Exception in HostFunction: [OP-SQLite] DB is not open' exception even thou I closes only the first connection.

Versions:

  • OS and version: Android 14 (SDK 34)
  • op-sqlite version: 7.4.3
  • RN version: 0.73.8
@ospfranco
Copy link
Contributor

No, this is a bug, it's unlikely you need two connections to the same db in your JS but I will take care of it anyways

@ospfranco
Copy link
Contributor

ospfranco commented Sep 18, 2024

Allowing multiple clean connections to the DB would require far more refactoring than I have time for. I have now disabled it. If you try to open a new connection to the same DB in JS it will throw an exception.

It will be released with the next major version.

#142

@On2it-Clifford
Copy link
Author

On2it-Clifford commented Sep 18, 2024

Hi @ospfranco, I understand what you meant. But the main reason I asked for this is because I have multiple backend jobs running at the same time. Example:
Scheduler A: Every 5 seconds will query Table A and do some task
Scheduler B: Every 5 seconds will query Table B and do some task

The problem is that occasionally, when both task run at the same time, one of the scheduler will closes the other scheduler's connection when it completes it task by calling .close, and this will cause the other scheduler that has yet to complete its task to have it's DB connection closed due to calling .close will close every single opened DB connection.

My example posted there is just to have a simple test case to replicate the issue that I am currently facing right now. My use case is far more complicated than the example I posted here.

@ospfranco
Copy link
Contributor

ospfranco commented Sep 18, 2024

You don't need multiple connections for this. Just use executeAsync, tasks run on separate threads. No need to open/close the db every single time.

@ospfranco
Copy link
Contributor

On the new version that will be released soon, you just need execute which is now async, there are no more sync versions.

@On2it-Clifford
Copy link
Author

On2it-Clifford commented Sep 18, 2024

Yes @ospfranco, initially I do not close db connection and reuse same db connection, it is working fine. But this will cause memory leak, where each query will consume the memory and does not release it if I do not call .close() and eventually it will crash the app when the memory is too low.

@On2it-Clifford
Copy link
Author

On2it-Clifford commented Sep 18, 2024

You can do a simple test where you open a db connection and use a while(true) loop that constantly query something(anything simple is applicable) using the same db connection without .close(), the memory never get released until we kill the app.

@ospfranco
Copy link
Contributor

Interesting, the new version should help maybe. Objects returned by the normal execute are no longer HostObjects so they can be properly cleaned. Give the 8.0.0-beta0 a try and if not open a new issue. No memory leaks should happen.

@On2it-Clifford
Copy link
Author

Alright @ospfranco I will test it out and let you know again, thanks.

@ospfranco
Copy link
Contributor

8.0.1 is out, give that one a try

@On2it-Clifford
Copy link
Author

Just had a quick test on 8.0.0-beta0, seems to be working very fine with no leaks at all. Will try with 8.0.1 again later. Thanks for the quick response.

@0cothucluc
Copy link

0cothucluc commented Nov 12, 2024

@ospfranco I'm facing [Error: Exception in HostFunction: [OP-SQLITE] Only one connection per database is allowed, db name: abc, when I open app again, it sometimes throw the exception, what is the best practice for this case?
image

@ospfranco
Copy link
Contributor

Don't close the connection, just create one per app start

@0cothucluc
Copy link

0cothucluc commented Nov 12, 2024

so I just need to open the connection when app starts right?
Edit: when I dont close the connection, the exception still there

@ospfranco
Copy link
Contributor

Read the docs, you also need to await the execute call

@0cothucluc
Copy link

I added .then() for execute call
The docs u just say use .close to close the connection

@ospfranco
Copy link
Contributor

Your app initialization code is wrong. When your app starts/hot reload is trying to create the DB connection again, which is wrong. I cannot tell you what's wrong because I don't have access to your code. Just create the connection once. Don't put it in a function just in a db.ts file create it at the module level:

// db.ts
export const db = open({...})

Read the docs carefully.

@0cothucluc
Copy link

I got it, I put .open in useEffect(), thx for fast reply

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants