CalebBell / thermo

Thermodynamics and Phase Equilibrium component of Chemical Engineering Design Library (ChEDL)
MIT License
634 stars 117 forks source link

SQLite Fail on threading when recursive use of cursors #156

Closed SomaLily closed 1 week ago

SomaLily commented 1 month ago

following #140 when 0.3.0 updated, SQLite objects created in a thread can only be used in that same thread issue had been done. But similar problem still alive. I changed the test_unifac_sql_connection_check_threading_false test method like this:

  def test_recursive_use_of_cursors():
    import threading
    from thermo import unifac
    unifac.init_ddbst_UNIFAC_db()
    assert unifac.UNIFAC_DDBST_ASSIGNMENT_CURSOR is not None

    def worker(raised_exception: threading.Event):
        try:
            unifac.UNIFAC_group_assignment_DDBST('50-14-6', 'UNIFAC')
            # one more query
            unifac.UNIFAC_group_assignment_DDBST('50-14-6', 'UNIFAC')
        except Exception as e:
            print(f'Thread {threading.current_thread().name}: Error: {e}')
            raised_exception.set()

    raised = threading.Event()

    threads = []
    for i in range(2):  # one more threading
        t = threading.Thread(target=worker, args=(raised,), name=f'Thread-{i+1}')
        threads.append(t)
        t.start()

    for t in threads:
        t.join()

    assert not raised.is_set()

then the Recursive use of cursors not allowed issue raises up, which is likely because thermo using a global UNIFAC_DDBST_ASSIGNMENT_CURSOR . Wondering if one could change it to local, may avoid this kind of SQLite issue ever ever.

CalebBell commented 1 month ago

Hi SomaLily, I can reproduce the issue. I understand the original fix didn't resolve root cause, which I didn't look into enough to understand in the first place. I agree a fix is necessary.

CalebBell commented 1 month ago

Hi SomaLily, I've written a fix for this issue by ensuring each thread has their own connection to any sqlite database. The performance overhead is very minimal. Thank you for the test case, I have added it to the test suite. The correction will be in the next thermo release. Sincerely, Caleb

SomaLily commented 3 weeks ago

Hi CalebBell, I'm glad to hear that! It would be helpful! I love thermo! Best regards, Soma

CalebBell commented 1 week ago

Hello, This fix landed in thermo 0.4.0 released today. Sincerely, Caleb