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

SQLite Fail on threading #140

Closed
cloasdata opened this issue Sep 13, 2023 · 4 comments
Closed

SQLite Fail on threading #140

cloasdata opened this issue Sep 13, 2023 · 4 comments

Comments

@cloasdata
Copy link

Hello Caleb,

it looks like unifac and its underlying sqlite database is making trouble when querying from different threads.
So far I could, see the database is read only and therefore it should be not a problem to allow different threads.

However, as the connection/cursor is stored global in the modul, it will only live once in the interpreter.
Ergo, when quering from different threads and different mixutres/chemicals, the singleton sqllite driver will sooner or later complain.

For now I used the follwing patch:

def patch_thermo_sqlite():
    """
    A simple monkey patch to make sqlite ignore threads.
    :return:
    """
    from thermo import unifac

    def init_db():
        import sqlite3
        import os
        conn = sqlite3.connect(
            os.path.join(
                os.path.dirname(unifac.__file__),
                'Phase Change',
                'DDBST_UNIFAC_assignments.sqlite'
            ),
            check_same_thread=False  # now driver will ignore different threads
        )
        unifac.UNIFAC_DDBST_ASSIGNMENT_CURSOR = conn.cursor()

    unifac.init_ddbst_UNIFAC_db = init_db

However, not sure why thermo loads UNIFAC when only dealing with methane. I guess it is because of mapping the CAS number to a chemical.

@cloasdata
Copy link
Author

Meanwhile I created a pull request. #141

@CalebBell
Copy link
Owner

Hi @cloasdata,
Interesting find! I hadn't encountered this issue before. All use of sqlite in thermo is currently read-only so the check_same_thread=False option is safe globally.

I appreciate your willingness to write a PR. In the spirit of keeping things simple, can you please reduce its scope to only that change without allowing arbitrary user options and having a test? I think this is best solved with the simple boolean change.

Sincerely,
Caleb

@cloasdata
Copy link
Author

Hello Caleb,

Understood.
I have updated the PR as requested. The test now uses threading explicit to see if an exception was thrown in the child thread.
I found this error while implementing dearpygui, a gui framework. DPG uses separate threads for state and callback management. And there this problems occurs. Sometimes you have luck and the callback is always done in the same thread but sometimes not and you get this weird error.
cheers Simon

@CalebBell
Copy link
Owner

Hello Simon,
That's cool to hear. Thanks for this fix! It'll be in the next release.
Sincerely,
Caleb

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

2 participants