fix: db lock #1069

.github/workflows/release.yml

@@ -13,7 +13,7 @@ jobs:
      - name: Set up Python
        uses: actions/setup-python@v2
        with:
-        python-version: '3.10'
+        python-version: '3.11'
      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip

.github/workflows/test.yml

@@ -15,7 +15,7 @@ jobs:
         id: setup-python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.9
+          python-version: 3.11
       - uses: actions/checkout@v2
       - name: Load cached venv
         id: cached-poetry-dependencies

psa_car_controller/psacc/application/battery_charge_curve.py

@@ -5,7 +5,6 @@ from typing import List
 from psa_car_controller.psacc.model.battery_curve import BatteryCurveDto
 from psa_car_controller.psacc.model.car import Car
 from psa_car_controller.psacc.model.charge import Charge
-from psa_car_controller.psacc.repository.db import Database
 
 DEFAULT_KM_BY_KW = 5.3
 MINIMUM_AUTONOMY_FOR_GOOD_RESULT = 20
@@ -21,8 +20,6 @@ class BatteryChargeCurve:
             -> 'List[BatteryChargeCurve]':  # pylint: disable=too-many-locals
         start_date = charge.start_at
         stop_at = charge.stop_at
-        conn = Database.get_db()
-        conn.close()
         battery_curves = []
         if len(battery_curves_dto) > 0 and battery_curves_dto[-1].level > 0 and battery_curves_dto[-1].autonomy > 0:
             battery_capacity = battery_curves_dto[-1].level * car.battery_power / 100

psa_car_controller/psacc/repository/db.py

@@ -39,26 +39,33 @@ class CustomSqliteConnection(sqlite3.Connection):
 
     def __init__(self, *args, **kwargs):  # real signature unknown
         super().__init__(*args, **kwargs)
-        self.callbacks = []
+        self.callbacks = set()
         self.execute("PRAGMA journal_mode=WAL;")
+        self.change_since_last_close = 0
 
     def execute_callbacks(self):
+        logger.debug("Executing callbacks")
         for callback in self.callbacks:
             callback()
+        logger.debug("End executing callbacks")
 
+    # we don't close the connection because it's shared between thread
     def close(self):
-        if self.total_changes:
+        if self.total_changes != self.change_since_last_close:
             self.execute_callbacks()
+            self.change_since_last_close = self.total_changes
         self.rollback()
         self.execute("PRAGMA optimize;")
         self.commit()
+
+    def close_db(self):
         super().close()
 
 
 class Database:
     callback_fct: Callable[[], None] = lambda: None
     DEFAULT_DB_FILE = 'info.db'
-    db_initialized = False
+    __conn = None
     __thread_lock = Lock()
 
     @staticmethod
@@ -75,7 +82,11 @@ class Database:
 
     @staticmethod
     def set_db_callback(callbackfct):
-        Database.callback_fct = callbackfct
+        with Database.__thread_lock:
+            if Database.__conn:
+                Database.__conn.callbacks.clear()
+                Database.__conn.callbacks.add(callbackfct)
+            Database.callback_fct = callbackfct
 
     @staticmethod
     def backup(conn):
@@ -139,17 +150,28 @@ class Database:
         return False
 
     @staticmethod
-    def get_db(db_file=None, update_callback=True) -> CustomSqliteConnection:
-        if db_file is None:
-            db_file = Database.DEFAULT_DB_FILE
-        conn = CustomSqliteConnection(db_file, detect_types=sqlite3.PARSE_DECLTYPES | sqlite3.PARSE_COLNAMES)
-        conn.row_factory = sqlite3.Row
+    def get_db(db_file=None, force_new_conn=False) -> CustomSqliteConnection:
+        assert sqlite3.threadsafety == 3, \
+            "SQLite is not in serialized mode (sqlite3.threadsafety != 3). " + \
+            "Upgrade python to python3.11"
         with Database.__thread_lock:
-            if not Database.db_initialized:
+            if not Database.__conn or force_new_conn:
+                if db_file is None:
+                    db_file = Database.DEFAULT_DB_FILE
+                conn = CustomSqliteConnection(db_file, detect_types=sqlite3.PARSE_DECLTYPES | sqlite3.PARSE_COLNAMES,
+                                              check_same_thread=False)
+                conn.row_factory = sqlite3.Row
                 Database.init_db(conn)
-        if update_callback:
-            conn.callbacks.append(Database.callback_fct)
-        return conn
+                Database.__conn = conn
+                logger.info("database initialized !")
+        Database.set_db_callback(Database.callback_fct)
+        return Database.__conn
+
+    @staticmethod
+    def close_db():
+        if Database.__conn:
+            Database.__conn.close_db()
+            Database.__conn = None
 
     @staticmethod
     def clean_battery(conn):
@@ -176,7 +198,6 @@ class Database:
         conn = Database.get_db()
         res = conn.execute("SELECT temperature FROM position WHERE VIN=? ORDER BY Timestamp DESC limit 1",
                            (vin,)).fetchone()
-        conn.close()
         if res is None:
             return None
         return res[0]
@@ -244,7 +265,6 @@ class Database:
                                           "level": row["level"], "level_fuel": row["level_fuel"]})
             features_list.append(feature)
         feature_collection = FeatureCollection(features_list)
-        conn.close()
         return geo_dumps(feature_collection, sort_keys=True)
 
     # pylint: disable=too-many-arguments,too-many-positional-arguments
@@ -253,31 +273,42 @@ class Database:
         if mileage == 0:  # fix a bug of the api
             logger.error("The api return a wrong mileage for %s : %f", vin, mileage)
         else:
-            conn = Database.get_db()
-            if conn.execute("SELECT Timestamp from position where Timestamp=?", (date,)).fetchone() is None:
-                temp = get_temp(latitude, longitude, weather_api)
-                if level_fuel and level_fuel == 0:  # fix fuel level not provided when car is off
-                    try:
-                        level_fuel = conn.execute(
-                            "SELECT level_fuel FROM position WHERE level_fuel>0 AND VIN=? ORDER BY Timestamp DESC "
-                            "LIMIT 1",
-                            (vin,)).fetchone()[0]
-                        logger.info("level_fuel fixed with last real value %f for %s", level_fuel, vin)
-                    except TypeError:
-                        level_fuel = None
-                        logger.info("level_fuel unfixed for %s", vin)
+            try:
+                conn = Database.get_db()
+                if conn.execute("SELECT Timestamp from position where Timestamp=?", (date,)).fetchone() is None:
+                    temp = get_temp(latitude, longitude, weather_api)
+                    if level_fuel and level_fuel == 0:  # fix fuel level not provided when car is off
+                        try:
+                            level_fuel = conn.execute(
+                                "SELECT level_fuel FROM position WHERE level_fuel>0 AND VIN=? ORDER BY Timestamp DESC "
+                                "LIMIT 1",
+                                (vin,)).fetchone()[0]
+                            logger.info("level_fuel fixed with last real value %f for %s", level_fuel, vin)
+                        except TypeError:
+                            level_fuel = None
+                            logger.info("level_fuel unfixed for %s", vin)
 
-                conn.execute("INSERT INTO position(Timestamp,VIN,longitude,latitude,altitude,mileage,level,level_fuel,"
-                             "moving,temperature) VALUES(?,?,?,?,?,?,?,?,?,?)",
-                             (date, vin, longitude, latitude, altitude, mileage, level, level_fuel, moving, temp))
+                    conn.execute(
+                        "INSERT INTO position(Timestamp,VIN,longitude,latitude,altitude,mileage,level,level_fuel,"
+                        "moving,temperature) VALUES(?,?,?,?,?,?,?,?,?,?)",
+                        (date,
+                         vin,
+                         longitude,
+                         latitude,
+                         altitude,
+                         mileage,
+                         level,
+                         level_fuel,
+                         moving,
+                         temp))
 
-                conn.commit()
-                logger.info("new position recorded for %s", vin)
-                Database.clean_position(conn)
+                    conn.commit()
+                    logger.info("new position recorded for %s", vin)
+                    Database.clean_position(conn)
+                    return True
+                logger.debug("position already saved")
+            finally:
                 conn.close()
-                return True
-            conn.close()
-            logger.debug("position already saved")
         return False
 
     @staticmethod
@@ -335,19 +366,20 @@ class Database:
     def get_all_charge() -> List[Charge]:
         conn = Database.get_db()
         res = conn.execute("select * from battery ORDER BY start_at").fetchall()
-        conn.close()
         return res
 
     @staticmethod
     def update_charge(charge: Charge):
         # we don't need to update mileage, since it should be inserted at beginning of charge,
         # maybe in future this will be supported
-        conn = Database.get_db()
-        res = conn.execute(
-            "UPDATE battery set stop_at=?, end_level=?, co2=?, kw=?, price=? WHERE start_at=? and VIN=?",
-            (charge.stop_at, charge.end_level, charge.co2, charge.kw, charge.price, charge.start_at,
-             charge.vin)).rowcount
-        if res == 0:
-            logger.error("Can't find battery row to update")
-        conn.commit()
-        conn.close()
+        try:
+            conn = Database.get_db()
+            res = conn.execute(
+                "UPDATE battery set stop_at=?, end_level=?, co2=?, kw=?, price=? WHERE start_at=? and VIN=?",
+                (charge.stop_at, charge.end_level, charge.co2, charge.kw, charge.price, charge.start_at,
+                 charge.vin)).rowcount
+            if res == 0:
+                logger.error("Can't find battery row to update")
+            conn.commit()
+        finally:
+            conn.close()

psa_car_controller/psacc/repository/trips.py

@@ -136,5 +136,4 @@ class Trips(list):
                             trip.add_points(end["latitude"], end["longitude"])
                     end = next_point
                 trips_by_vin[vin] = trips
-        conn.close()
         return trips_by_vin

psa_car_controller/web/figures.py

@@ -221,5 +221,4 @@ def get_altitude_fig(trip: Trip):
         line[0] = line[0] - start_mileage
     fig = px.line(res, x=0, y=1)
     fig.update_layout(xaxis_title="Distance km", yaxis_title="Altitude m")
-    conn.close()
     return html.Div(Graph(figure=fig))

psa_car_controller/web/view/views.py

@@ -178,9 +178,8 @@ def create_callback():  # noqa: MC0001
 def update_trips():
     global trips, chargings, cached_layout, min_date, max_date, min_millis, max_millis, step, marks
     logger.info("update_data")
-    conn = Database.get_db(update_callback=False)
+    conn = Database.get_db()
     Database.add_altitude_to_db(conn)
-    conn.close()
     min_date = None
     max_date = None
     if APP.is_good:

pyproject.toml

@@ -11,7 +11,7 @@ include = [
 ]
 
 [tool.poetry.dependencies]
-python = ">=3.9, <4.0.0"
+python = ">=3.11, <4.0.0"
 paho-mqtt = ">=1.5.0, <2.0.0"
 dash = ">=2.9.0, <3.0.0"
 dash-daq = "^0.5.0"

tests/data/car_status.py

@@ -39,7 +39,7 @@ ELECTRIC_CAR_STATUS = {
     "odometer": {"createdAt": None, "mileage": 3196.5}, "updatedAt": "2022-03-26T11:02:54Z"}
 ELECTRIC_CAR_STATUS_V2 = {
     "lastPosition": {"type": "Feature", "geometry": {"type": "Point", "coordinates": [-1.59008, 47.274, 30]},
-                     "properties": {"updatedAt": "2021-03-29T06:22:51Z", "type": "Acquire", "signalQuality": 9}},
+                     "properties": {"createdAt": "2021-03-29T06:22:51Z", "type": "Acquire", "signalQuality": 9}},
     "preconditionning": {"airConditioning": {"updatedAt": "2022-03-26T10:52:11Z", "status": "Disabled"}},
     "energy": [{"createdAt": "2021-09-14T20:39:06Z", "type": "Fuel", "level": 0},
                {"updatedAt": "2022-03-26T11:02:54Z", "type": "Electric", "level": 59, "autonomy": 122,

tests/utils.py

@@ -28,17 +28,11 @@ vehicule_list = Cars()
 vehicule_list.extend(
     [Car("VR3UHZKX", "vid", "Peugeot"), Car("VXXXXX", "XXXX", "Peugeot", label="SUV 3008 Hybrid 225")])
 car = vehicule_list[0]
-DB_DIR = DATA_DIR + "tmp.db"
 
 
 def get_new_test_db():
-    try:
-        os.remove(DATA_DIR + "tmp.db")
-    except FileNotFoundError:
-        pass
-    Database.DEFAULT_DB_FILE = DB_DIR
-    Database.db_initialized = False
-    conn = Database.get_db()
+    Database.close_db()
+    conn = Database.get_db(force_new_conn=True, db_file="")
     return conn