From bab50299d851751d1f037f7df802b06341855d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20F=C3=A4ssler?= Date: Sun, 15 Sep 2013 16:00:16 +0000 Subject: [PATCH] Fix thread safety for database cursor The database cursor is not thread save. That's why uploading multiple tracks at once sometimes failed. Fixes #2 Fixes #3 Reported-By: cleanerx --- src/auth.py | 41 ++++++++++++++++++++++++----------------- src/track.py | 36 ++++++++++++++++++++---------------- src/vessel.py | 41 ++++++++++++++++++++++++----------------- 3 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/auth.py b/src/auth.py index 402c435..a08b89a 100644 --- a/src/auth.py +++ b/src/auth.py @@ -40,25 +40,20 @@ class Auth(): def __init__(self, db): self._db = db - self._cur = db.getCursor() self._maxUnsuccessfulAttempts = 3 self._blockForMinutes = 3 # ---------------------------------------------------------------------------------------------- - def __del__(self): - self._cur.close() - - # ---------------------------------------------------------------------------------------------- - def authenticate(self, username, password): ''' Authenticates user account. The account will be blocked for '_blockForMinutes' if the maximum attempts '_maxUnsuccessfulAttempts' is reached. ''' username = self.hashUsername(username) - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT password, salt, attempts, last_attempt FROM user_profiles @@ -66,8 +61,9 @@ def authenticate(self, username, password): ''', (username,) ) - record = self._cur.fetchone() + record = cursor.fetchone() self._db.commit() + cursor.close() if record == None: raise Error(101, 'Username or password wrong.', 'AUTH') if record[2] >= self._maxUnsuccessfulAttempts: @@ -90,7 +86,8 @@ def authenticate(self, username, password): def create(self, username, password): username = self.hashUsername(username) - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT count(*) FROM user_profiles @@ -98,11 +95,12 @@ def create(self, username, password): ''', (username,) ) - record = self._cur.fetchone() - self._db.commit() + record = cursor.fetchone() if record[0] > 0: + self._db.commit() + cursor.close() raise Error(103, 'Username already exists.', 'AUTH') - self._cur.execute( + cursor.execute( ''' INSERT INTO user_profiles (user_name, password, salt) @@ -111,13 +109,15 @@ def create(self, username, password): (username,) ) self._db.commit() + cursor.close() self._setPassword(username, password) # ---------------------------------------------------------------------------------------------- def changePassword(self, username, oldPassword, newPassword): username = self.hashUsername(username) - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT password, salt FROM user_profiles @@ -125,8 +125,9 @@ def changePassword(self, username, oldPassword, newPassword): ''', (username,) ) - record = self._cur.fetchone() + record = cursor.fetchone() self._db.commit() + cursor.close() if record == None: raise Error(101, 'Username or password wrong.', 'AUTH') saltedPassword = self._saltPassword(oldPassword, record[1]) @@ -148,7 +149,8 @@ def hashUsername(self, username): def _setPassword(self, username, password): salt = self._getNewSalt() saltedPassword = self._saltPassword(password, salt) - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' UPDATE user_profiles SET password = %s, salt = %s @@ -157,6 +159,7 @@ def _setPassword(self, username, password): (saltedPassword, salt, username) ) self._db.commit() + cursor.close() # ---------------------------------------------------------------------------------------------- @@ -179,7 +182,8 @@ def _saltPassword(self, password, salt): # ---------------------------------------------------------------------------------------------- def _updateAttempts(self, username, attempts, lastAttempt): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' UPDATE user_profiles SET attempts = %s, last_attempt = %s @@ -188,14 +192,17 @@ def _updateAttempts(self, username, attempts, lastAttempt): (attempts, lastAttempt, username) ) self._db.commit() + cursor.close() # ---------------------------------------------------------------------------------------------- def removeAllProfiles(self): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' DELETE FROM user_profiles ''' ) self._db.commit() + cursor.close() diff --git a/src/track.py b/src/track.py index c318fb7..bd26c2a 100644 --- a/src/track.py +++ b/src/track.py @@ -56,7 +56,6 @@ class Track(): def __init__(self, db): self._db = db - self._cur = db.getCursor() try: self._dir = cherrypy.config.get('track.dir', None) @@ -68,23 +67,20 @@ def __init__(self, db): # ---------------------------------------------------------------------------------------------- - def __del__(self): - self._cur.close() - - # ---------------------------------------------------------------------------------------------- - def getNewId(self, username): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT nextval('user_tracks_track_id_seq') ''' ) - record = self._cur.fetchone() - self._db.commit() + record = cursor.fetchone() if record == None: + self._db.commit() + cursor.close() raise Error(201, 'Unable to fetch new track id.', 'TRACK') trackId = record[0] - self._cur.execute( + cursor.execute( ''' INSERT INTO user_tracks (track_id, user_name) VALUES (%s, %s) @@ -92,6 +88,7 @@ def getNewId(self, username): (trackId, username) ) self._db.commit() + cursor.close() return trackId # ---------------------------------------------------------------------------------------------- @@ -99,7 +96,8 @@ def getNewId(self, username): def uploadDone(self, trackId, username, fileHandle, fileName): assert isinstance(trackId, int) assert isinstance(fileHandle, file) - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT upload_state FROM user_tracks @@ -108,16 +106,19 @@ def uploadDone(self, trackId, username, fileHandle, fileName): ''', (trackId, username) ) - record = self._cur.fetchone() - self._db.commit() + record = cursor.fetchone() if record == None: + self._db.commit() + cursor.close() raise Error(202, 'Invalid track id.', 'TRACK') if record[0] > 0: + self._db.commit() + cursor.close() raise Error(203, 'Upload already done.', 'TRACK') size = self._storeFile(trackId, fileHandle) - self._cur.execute( + cursor.execute( ''' UPDATE user_tracks SET file_ref = %s, upload_state = 1 @@ -126,6 +127,7 @@ def uploadDone(self, trackId, username, fileHandle, fileName): (fileName, trackId) ) self._db.commit() + cursor.close() return size @@ -158,7 +160,8 @@ def _generateFileName(self, trackId): # ---------------------------------------------------------------------------------------------- def getByUsername(self, username): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT track_id, file_ref, upload_state FROM user_tracks @@ -167,6 +170,7 @@ def getByUsername(self, username): ''', (username,) ) - records = self._cur.fetchall() + records = cursor.fetchall() self._db.commit() + cursor.close() return records diff --git a/src/vessel.py b/src/vessel.py index e8d205b..ad46b99 100644 --- a/src/vessel.py +++ b/src/vessel.py @@ -52,19 +52,14 @@ class Vessel(): def __init__(self, db): self._db = db - self._cur = db.getCursor() - - # ---------------------------------------------------------------------------------------------- - - def __del__(self): - self._cur.close() # ---------------------------------------------------------------------------------------------- def create(self, username, name): assert len(username) > 0 assert len(name) > 0 - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT count(*) FROM user_vessels @@ -73,11 +68,12 @@ def create(self, username, name): ''', (username, name) ) - record = self._cur.fetchone() + record = cursor.fetchone() if record[0] > 0: self._db.commit() + cursor.close() raise Error(204, 'Vessel name already exists.', 'VESSEL') - self._cur.execute( + cursor.execute( ''' INSERT INTO user_vessels (user_name, settings) VALUES (%s, hstore('name' => %s)) @@ -85,24 +81,27 @@ def create(self, username, name): ''', (username, name) ) - record = self._cur.fetchone() + record = cursor.fetchone() vesselId = record[0] self._db.commit() + cursor.close() return vesselId # ---------------------------------------------------------------------------------------------- def setParams(self, username, vesselId, params): + cursor = self._db.getCursor() for param, value in params.iteritems(): - self._setParam(username, vesselId, param, value) + self._setParam(cursor, username, vesselId, param, value) self._db.commit() + cursor.close() # ---------------------------------------------------------------------------------------------- - def _setParam(self, username, vesselId, param, value): + def _setParam(self, cursor, username, vesselId, param, value): self._valueValid(param, value) try: - self._cur.execute( + cursor.execute( ''' UPDATE user_vessels SET settings = settings || (%s => %s) @@ -112,9 +111,13 @@ def _setParam(self, username, vesselId, param, value): ''', (param, str(value), vesselId, username) ) - if self._cur.fetchone() == None: + if cursor.fetchone() == None: + self._db.commit() + cursor.close() raise Error(207, 'Invalid vessel ID.', 'VESSEL') except psycopg2.IntegrityError as e: + self._db.commit() + cursor.close() raise Error(204, 'Vessel name already exists.', 'VESSEL') # ---------------------------------------------------------------------------------------------- @@ -144,7 +147,8 @@ def _valueValid(self, param, value): # ---------------------------------------------------------------------------------------------- def getByUsername(self, username): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' SELECT vessel_id, settings FROM user_vessels @@ -154,18 +158,21 @@ def getByUsername(self, username): (username,) ) result = [] - for vesselId, settings in self._cur: + for vesselId, settings in cursor: settings['vesselId'] = vesselId result.append(settings) self._db.commit() + cursor.close() return result # ---------------------------------------------------------------------------------------------- def removeAll(self): - self._cur.execute( + cursor = self._db.getCursor() + cursor.execute( ''' DELETE FROM user_vessels ''' ) self._db.commit() + cursor.close()