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,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

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.

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(),

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(),

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

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 ):
     """

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