Willy-JL / F95Checker

GNU General Public License v3.0
113 stars 17 forks source link

shouldn't the db save loop be cancelled? #25

Closed dcnieho closed 1 year ago

dcnieho commented 2 years ago

It is theoretically possible to have the save action executed after the db connection is closed but before the application is fully exited. Shouldn't you do something like this to guard against that?

diff --git a/main.py b/main.py
index a37c7fc..1e8255e 100755
--- a/main.py
+++ b/main.py
@@ -45,7 +45,7 @@ def main():
     from modules import db
     async_thread.wait(db.connect())
     async_thread.wait(db.load())
-    async_thread.run(db.save_loop())
+    db_save_future = async_thread.run(db.save_loop())

     from modules import api
     with api.setup():
@@ -59,6 +59,7 @@ def main():

         globals.gui.main_loop()

+    db_save_future.cancel()
     async_thread.wait(db.close())
     async_thread.wait(api.shutdown())
Willy-JL commented 2 years ago

Little bit of a race condition huh? Indeed this can be a problem, thanks for the bug report! Btw for stuff like this a pull request allows you to submit (or suggest) changes to the code base and also get credit in the homepage and release page!

dcnieho commented 2 years ago

I wasn't so sure as on the one hand threading logic tells me this should be an issue, on the other hand, i am hardly familiar with Python async, nor am i much of a Python programmer. So i wasn't sure this was the right solution, nor if there are nooks and crannies to Python's async that make this not an issue. All that said, given the topic of your software i prefer not to get too much credit haha. I'm already working from home to avoid the employer having questions ;)

On Wed, Sep 7, 2022 at 3:58 PM WillyJL @.***> wrote:

Little bit of a race condition huh? Indeed this can be a problem, thanks for the bug report! Btw for stuff like this a pull request allows you to submit (or suggest) changes to the code base and also get credit in the homepage and release page!

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

Willy-JL commented 2 years ago

Ah of course :D

Also side note this made me realize the the following api.shutdown is superfluous now, since it’s handled by the with block