From f3b0d563c18deb6b1fd6552c401187d6dc49d268 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 17 Mar 2008 21:17:00 +0000 Subject: [PATCH] Fixed database transaction leak by wrapping every exposed database-using controller method with a new @end_transaction() decorator. This decorator is responsible for rolling back unfinished transactions. --- NEWS | 1 + controller/Database.py | 15 +++++++++++++++ controller/Files.py | 9 ++++++++- controller/Notebooks.py | 21 ++++++++++++++++++++- controller/Root.py | 10 +++++++++- controller/Users.py | 14 +++++++++++++- controller/test/Stub_database.py | 3 +++ 7 files changed, 69 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 0fe3751..c62fec0 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ 1.2.14: March ??, 2008 * Added ability to reorder notebooks on the right side of the page. + * Fixed database transaction leak that was wasting memory. 1.2.13: March 11, 2008 * When the "all notes" note is the only note open, it now actually hides when diff --git a/controller/Database.py b/controller/Database.py index 2aee531..17494a9 100644 --- a/controller/Database.py +++ b/controller/Database.py @@ -1,6 +1,7 @@ import re import os import sha +import cherrypy import psycopg2 as psycopg from psycopg2.pool import PersistentConnectionPool import random @@ -320,6 +321,20 @@ class Database( object ): self.__pool.closeall() +def end_transaction( function ): + """ + Decorator that prevents transaction leaks by rolling back any transactions left open when the + wrapped function returns or raises. + """ + def rollback( *args, **kwargs ): + try: + return function( *args, **kwargs ) + finally: + cherrypy.root.database.rollback() + + return rollback + + class Valid_id( object ): """ Validator for an object id. diff --git a/controller/Files.py b/controller/Files.py index 87255ae..4892892 100644 --- a/controller/Files.py +++ b/controller/Files.py @@ -7,7 +7,7 @@ import cherrypy from threading import Lock, Event from Expose import expose from Validate import validate, Valid_int, Validation_error -from Database import Valid_id +from Database import Valid_id, end_transaction from Users import grab_user_id from Expire import strongly_expire from model.File import File @@ -232,6 +232,7 @@ class Files( object ): self.__users = users @expose() + @end_transaction @grab_user_id @validate( file_id = Valid_id(), @@ -280,6 +281,7 @@ class Files( object ): @expose( view = Upload_page ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -314,6 +316,7 @@ class Files( object ): @expose( view = Blank_page ) @strongly_expire + @end_transaction @grab_user_id @validate( upload = (), @@ -383,6 +386,7 @@ class Files( object ): @expose() @strongly_expire + @end_transaction @grab_user_id @validate( file_id = Valid_id(), @@ -449,6 +453,7 @@ class Files( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( file_id = Valid_id(), @@ -487,6 +492,7 @@ class Files( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( file_id = Valid_id(), @@ -523,6 +529,7 @@ class Files( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( file_id = Valid_id(), diff --git a/controller/Notebooks.py b/controller/Notebooks.py index 00d6ffb..cc6a5f3 100644 --- a/controller/Notebooks.py +++ b/controller/Notebooks.py @@ -3,7 +3,7 @@ import cherrypy from datetime import datetime from Expose import expose from Validate import validate, Valid_string, Validation_error, Valid_bool -from Database import Valid_id, Valid_revision +from Database import Valid_id, Valid_revision, end_transaction from Users import grab_user_id from Expire import strongly_expire from Html_nuker import Html_nuker @@ -55,6 +55,7 @@ class Notebooks( object ): @expose( view = Main_page ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -207,6 +208,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -267,6 +269,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -358,6 +361,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -395,6 +399,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -445,6 +450,7 @@ class Notebooks( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -561,6 +567,7 @@ class Notebooks( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -614,6 +621,7 @@ class Notebooks( object ): return dict( storage_bytes = 0 ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -670,6 +678,7 @@ class Notebooks( object ): return dict( storage_bytes = 0 ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -722,6 +731,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -770,6 +780,7 @@ class Notebooks( object ): @expose( view = Json ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -804,6 +815,7 @@ class Notebooks( object ): @expose( view = Html_file ) @strongly_expire + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -839,6 +851,7 @@ class Notebooks( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -885,6 +898,7 @@ class Notebooks( object ): return notebook @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -936,6 +950,7 @@ class Notebooks( object ): return dict() @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -991,6 +1006,7 @@ class Notebooks( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -1033,6 +1049,7 @@ class Notebooks( object ): return dict( storage_bytes = user.storage_bytes ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -1073,6 +1090,7 @@ class Notebooks( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -1144,6 +1162,7 @@ class Notebooks( object ): return dict() @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), diff --git a/controller/Root.py b/controller/Root.py index b87abe7..2df7657 100644 --- a/controller/Root.py +++ b/controller/Root.py @@ -6,7 +6,7 @@ from Validate import validate, Valid_int, Valid_string from Notebooks import Notebooks from Users import Users, grab_user_id from Files import Files -from Database import Valid_id +from Database import Valid_id, end_transaction from model.Note import Note from model.Notebook import Notebook from model.User import User @@ -50,6 +50,7 @@ class Root( object ): self.__suppress_exceptions = suppress_exceptions # used for unit tests @expose( Main_page ) + @end_transaction @grab_user_id @validate( note_title = unicode, @@ -136,6 +137,7 @@ class Root( object ): @expose( view = Front_page ) @strongly_expire + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -172,6 +174,7 @@ class Root( object ): return result @expose( view = Tour_page ) + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -191,6 +194,7 @@ class Root( object ): return dict( redirect = u"/tour" ) @expose( view = Main_page, rss = Notebook_rss ) + @end_transaction @grab_user_id @validate( start = Valid_int( min = 0 ), @@ -225,6 +229,7 @@ class Root( object ): return result @expose( view = Main_page ) + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -245,6 +250,7 @@ class Root( object ): return result @expose( view = Main_page ) + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -266,6 +272,7 @@ class Root( object ): @expose( view = Main_page ) @strongly_expire + @end_transaction @grab_user_id @validate( user_id = Valid_id( none_okay = True ), @@ -307,6 +314,7 @@ class Root( object ): # TODO: move this method to controller.Notebooks, and maybe give it a more sensible name @expose( view = Json ) + @end_transaction def next_id( self ): """ Return the next available database object id for a new note. This id is guaranteed to be unique diff --git a/controller/Users.py b/controller/Users.py index 9444604..8d74a6d 100644 --- a/controller/Users.py +++ b/controller/Users.py @@ -11,7 +11,7 @@ from model.Password_reset import Password_reset from model.Invite import Invite from Expose import expose from Validate import validate, Valid_string, Valid_bool, Validation_error -from Database import Valid_id +from Database import Valid_id, end_transaction from Expire import strongly_expire from view.Json import Json from view.Main_page import Main_page @@ -196,6 +196,7 @@ class Users( object ): self.__rate_plans = rate_plans @expose( view = Json ) + @end_transaction @update_auth @validate( username = ( Valid_string( min = 1, max = 30 ), valid_username ), @@ -284,6 +285,7 @@ class Users( object ): ) @expose() + @end_transaction @grab_user_id @update_auth def demo( self, user_id = None ): @@ -347,6 +349,7 @@ class Users( object ): ) @expose( view = Json ) + @end_transaction @update_auth @validate( username = ( Valid_string( min = 1, max = 30 ), valid_username ), @@ -403,6 +406,7 @@ class Users( object ): ) @expose() + @end_transaction @update_auth def logout( self ): """ @@ -528,6 +532,7 @@ class Users( object ): return False @expose( view = Json ) + @end_transaction @validate( email_address = ( Valid_string( min = 1, max = 60 ), valid_email_address ), send_reset_button = unicode, @@ -585,6 +590,7 @@ class Users( object ): @expose( view = Main_page ) @strongly_expire + @end_transaction @validate( password_reset_id = Valid_id(), ) @@ -636,6 +642,7 @@ class Users( object ): return result @expose( view = Json ) + @end_transaction def reset_password( self, password_reset_id, reset_button, **new_passwords ): """ Reset all the users with the provided passwords. @@ -705,6 +712,7 @@ class Users( object ): return dict( redirect = u"/" ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -836,6 +844,7 @@ class Users( object ): ) @expose( view = Json ) + @end_transaction @grab_user_id @validate( notebook_id = Valid_id(), @@ -880,6 +889,7 @@ class Users( object ): ) @expose( view = Main_page ) + @end_transaction @grab_user_id @validate( invite_id = Valid_id(), @@ -979,6 +989,7 @@ class Users( object ): self.__database.commit() @expose( view = Blank_page ) + @end_transaction def paypal_notify( self, **params ): """ Notify Luminotes of payments, subscriptions, cancellations, refunds, etc. @@ -1082,6 +1093,7 @@ class Users( object ): return dict() @expose( view = Main_page ) + @end_transaction @grab_user_id def thanks( self, **params ): """ diff --git a/controller/test/Stub_database.py b/controller/test/Stub_database.py index f534e0b..1f79199 100644 --- a/controller/test/Stub_database.py +++ b/controller/test/Stub_database.py @@ -77,5 +77,8 @@ class Stub_database( object ): def commit( self ): pass + def rollback( self ): + pass + def close( self ): pass