Patchers,

Here is a small patch to refactor common functionality out of
LockRelease and LockReleaseAll, creating a new static function
RemoveProcLock.

(This comes from Heikki's two-phase commit patch, where it is used more.)


Additionally, I found that no callers of LockReleaseAll and LockRelease
had any use for their return values, so I made them return void.

There are two exceptions to the "no use of return value" assertion: one
is the userlock contrib module, but I doubt it has much use for it in
reality;  I made it "return true" where it used LockRelease{,All} return
value, in order not change the function's signature; users have no way
to recover when user_unlock_all does not work anyway, so we are not
losing anything.

The other exception is LockReleaseCurrentOwner, where the only action
is to raise a WARNING.  Since LockRelease already raises WARNING for
anything that returns a FALSE value, we don't lose anything by just
returning void and dropping the warning in ResourceOwners.

Please apply if there are no objections.

-- 
Alvaro Herrera (<alvherre[a]surnet.cl>)
"La libertad es como el dinero; el que no la sabe emplear la pierde" (Alvarez)
Index: src/include/storage/lock.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/storage/lock.h,v
retrieving revision 1.85
diff -c -r1.85 lock.h
*** src/include/storage/lock.h  29 Apr 2005 22:28:24 -0000      1.85
--- src/include/storage/lock.h  13 May 2005 01:05:43 -0000
***************
*** 359,367 ****
  extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid);
  extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode, bool dontWait);
! extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode);
! extern bool LockReleaseAll(LOCKMETHODID lockmethodid, bool allxids);
  extern void LockReleaseCurrentOwner(void);
  extern void LockReassignCurrentOwner(void);
  extern int LockCheckConflicts(LockMethod lockMethodTable,
--- 359,367 ----
  extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid);
  extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode, bool dontWait);
! extern void LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode);
! extern void LockReleaseAll(LOCKMETHODID lockmethodid, bool allxids);
  extern void LockReleaseCurrentOwner(void);
  extern void LockReassignCurrentOwner(void);
  extern int LockCheckConflicts(LockMethod lockMethodTable,
Index: src/backend/storage/lmgr/lock.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.151
diff -c -r1.151 lock.c
*** src/backend/storage/lmgr/lock.c     11 May 2005 01:26:02 -0000      1.151
--- src/backend/storage/lmgr/lock.c     13 May 2005 01:05:23 -0000
***************
*** 166,171 ****
--- 166,172 ----
                                 int *myHolding);
  static bool UnGrantLock(LOCK *lock, LOCKMODE lockmode,
                                                PROCLOCK *proclock, LockMethod 
lockMethodTable);
+ static bool RemoveProcLock(LOCKMETHODID lockmethodid, PROCLOCK *proclock, 
LOCK *lock);
  
  
  /*
***************
*** 792,797 ****
--- 793,840 ----
  }
  
  /*
+  * Subroutine to unlink and free a proclock entry, and garbage 
+  * collect the lock object.
+  *
+  * The locktable's masterLock must be held at entry, and will be
+  * held at exit.
+  */
+ static bool
+ RemoveProcLock(LOCKMETHODID lockmethodid, PROCLOCK *proclock, LOCK *lock)
+ {
+       PROCLOCK_PRINT("RemoveProcLock: deleting", proclock);
+       SHMQueueDelete(&proclock->lockLink);
+       SHMQueueDelete(&proclock->procLink);
+       proclock = (PROCLOCK *) 
hash_search(LockMethodProcLockHash[lockmethodid],
+                                                                               
(void *) &(proclock->tag),
+                                                                               
HASH_REMOVE, NULL);
+       if (!proclock)
+       {
+               elog(WARNING, "proclock table corrupted");
+               return FALSE;
+       }
+ 
+       if (lock->nRequested == 0)
+       {
+               /*
+                * We've just released the last lock, so garbage-collect the
+                * lock object.
+                */
+               LOCK_PRINT("RemoveProcLock: deleting", lock, 0);
+               Assert(SHMQueueEmpty(&(lock->procLocks)));
+               lock = (LOCK *) hash_search(LockMethodLockHash[lockmethodid],
+                                                                       (void 
*) &(lock->tag),
+                                                                       
HASH_REMOVE, NULL);
+               if (!lock)
+               {
+                       elog(WARNING, "lock table corrupted");
+                       return FALSE;
+               }
+       }
+       return TRUE;
+ }
+ 
+ /*
   * LockCheckConflicts -- test whether requested lock conflicts
   *            with those already granted
   *
***************
*** 1196,1202 ****
   *            the waking process and any new process to
   *            come along and request the lock.)
   */
! bool
  LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode)
  {
--- 1239,1245 ----
   *            the waking process and any new process to
   *            come along and request the lock.)
   */
! void
  LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag,
                        TransactionId xid, LOCKMODE lockmode)
  {
***************
*** 1223,1229 ****
        if (!lockMethodTable)
        {
                elog(WARNING, "lockMethodTable is null in LockRelease");
!               return FALSE;
        }
  
        /*
--- 1266,1272 ----
        if (!lockMethodTable)
        {
                elog(WARNING, "lockMethodTable is null in LockRelease");
!               return;
        }
  
        /*
***************
*** 1246,1252 ****
        {
                elog(WARNING, "you don't own a lock of type %s",
                         lock_mode_names[lockmode]);
!               return FALSE;
        }
  
        /*
--- 1289,1295 ----
        {
                elog(WARNING, "you don't own a lock of type %s",
                         lock_mode_names[lockmode]);
!               return;
        }
  
        /*
***************
*** 1284,1290 ****
                        /* don't release a lock belonging to another owner */
                        elog(WARNING, "you don't own a lock of type %s",
                                 lock_mode_names[lockmode]);
!                       return FALSE;
                }
        }
  
--- 1327,1333 ----
                        /* don't release a lock belonging to another owner */
                        elog(WARNING, "you don't own a lock of type %s",
                                 lock_mode_names[lockmode]);
!                       return;
                }
        }
  
***************
*** 1295,1301 ****
        locallock->nLocks--;
  
        if (locallock->nLocks > 0)
!               return TRUE;
  
        /*
         * Otherwise we've got to mess with the shared lock table.
--- 1338,1344 ----
        locallock->nLocks--;
  
        if (locallock->nLocks > 0)
!               return;
  
        /*
         * Otherwise we've got to mess with the shared lock table.
***************
*** 1325,1331 ****
                elog(WARNING, "you don't own a lock of type %s",
                         lock_mode_names[lockmode]);
                RemoveLocalLock(locallock);
!               return FALSE;
        }
  
        wakeupNeeded = UnGrantLock(lock, lockmode, proclock, lockMethodTable);
--- 1368,1374 ----
                elog(WARNING, "you don't own a lock of type %s",
                         lock_mode_names[lockmode]);
                RemoveLocalLock(locallock);
!               return;
        }
  
        wakeupNeeded = UnGrantLock(lock, lockmode, proclock, lockMethodTable);
***************
*** 1334,1388 ****
         * If this was my last hold on this lock, delete my entry in the
         * proclock table.
         */
!       if (proclock->holdMask == 0)
        {
!               PROCLOCK_PRINT("LockRelease: deleting proclock", proclock);
!               SHMQueueDelete(&proclock->lockLink);
!               SHMQueueDelete(&proclock->procLink);
!               proclock = (PROCLOCK *) 
hash_search(LockMethodProcLockHash[lockmethodid],
!                                                                               
        (void *) &(proclock->tag),
!                                                                               
        HASH_REMOVE, NULL);
!               if (!proclock)
!               {
!                       LWLockRelease(masterLock);
!                       elog(WARNING, "proclock table corrupted");
!                       RemoveLocalLock(locallock);
!                       return FALSE;
!               }
        }
  
!       if (lock->nRequested == 0)
!       {
!               /*
!                * We've just released the last lock, so garbage-collect the 
lock
!                * object.
!                */
!               LOCK_PRINT("LockRelease: deleting lock", lock, lockmode);
!               Assert(SHMQueueEmpty(&(lock->procLocks)));
!               lock = (LOCK *) hash_search(LockMethodLockHash[lockmethodid],
!                                                                       (void 
*) &(lock->tag),
!                                                                       
HASH_REMOVE, NULL);
!               if (!lock)
!               {
!                       LWLockRelease(masterLock);
!                       elog(WARNING, "lock table corrupted");
!                       RemoveLocalLock(locallock);
!                       return FALSE;
!               }
!       }
!       else
!       {
!               /*
!                * Wake up waiters if needed.
!                */
!               if (wakeupNeeded)
!                       ProcLockWakeup(lockMethodTable, lock);
!       }
  
        LWLockRelease(masterLock);
  
        RemoveLocalLock(locallock);
!       return TRUE;
  }
  
  /*
--- 1377,1400 ----
         * If this was my last hold on this lock, delete my entry in the
         * proclock table.
         */
!       if (proclock->holdMask == 0 &&
!               !RemoveProcLock(lockmethodid, proclock, lock))
        {
!               LWLockRelease(masterLock);
!               RemoveLocalLock(locallock);
!               return;
        }
  
!       /*
!        * Wake up waiters if needed.
!        */
!       if (wakeupNeeded)
!               ProcLockWakeup(lockMethodTable, lock);
  
        LWLockRelease(masterLock);
  
        RemoveLocalLock(locallock);
!       return;
  }
  
  /*
***************
*** 1397,1403 ****
   * allxids == false: release all locks with Xid != 0
   * (zero is the Xid used for "session" locks).
   */
! bool
  LockReleaseAll(LOCKMETHODID lockmethodid, bool allxids)
  {
        HASH_SEQ_STATUS status;
--- 1409,1415 ----
   * allxids == false: release all locks with Xid != 0
   * (zero is the Xid used for "session" locks).
   */
! void
  LockReleaseAll(LOCKMETHODID lockmethodid, bool allxids)
  {
        HASH_SEQ_STATUS status;
***************
*** 1420,1426 ****
        if (!lockMethodTable)
        {
                elog(WARNING, "bad lock method: %d", lockmethodid);
!               return FALSE;
        }
  
        numLockModes = lockMethodTable->numLockModes;
--- 1432,1438 ----
        if (!lockMethodTable)
        {
                elog(WARNING, "bad lock method: %d", lockmethodid);
!               return;
        }
  
        numLockModes = lockMethodTable->numLockModes;
***************
*** 1518,1562 ****
  
                PROCLOCK_PRINT("LockReleaseAll: deleting", proclock);
  
!               /*
!                * Remove the proclock entry from the linked lists
!                */
!               SHMQueueDelete(&proclock->lockLink);
!               SHMQueueDelete(&proclock->procLink);
  
!               /*
!                * remove the proclock entry from the hashtable
!                */
!               proclock = (PROCLOCK *) 
hash_search(LockMethodProcLockHash[lockmethodid],
!                                                                               
        (void *) &(proclock->tag),
!                                                                               
        HASH_REMOVE,
!                                                                               
        NULL);
!               if (!proclock)
                {
                        LWLockRelease(masterLock);
!                       elog(WARNING, "proclock table corrupted");
!                       return FALSE;
                }
  
!               if (lock->nRequested == 0)
!               {
!                       /*
!                        * We've just released the last lock, so 
garbage-collect the
!                        * lock object.
!                        */
!                       LOCK_PRINT("LockReleaseAll: deleting", lock, 0);
!                       Assert(SHMQueueEmpty(&(lock->procLocks)));
!                       lock = (LOCK *) 
hash_search(LockMethodLockHash[lockmethodid],
!                                                                               
(void *) &(lock->tag),
!                                                                               
HASH_REMOVE, NULL);
!                       if (!lock)
!                       {
!                               LWLockRelease(masterLock);
!                               elog(WARNING, "lock table corrupted");
!                               return FALSE;
!                       }
!               }
!               else if (wakeupNeeded)
                        ProcLockWakeup(lockMethodTable, lock);
  
  next_item:
--- 1530,1544 ----
  
                PROCLOCK_PRINT("LockReleaseAll: deleting", proclock);
  
!               Assert(proclock->holdMask == 0);
  
!               if (!RemoveProcLock(lockmethodid, proclock, lock))
                {
                        LWLockRelease(masterLock);
!                       return;
                }
  
!               if (wakeupNeeded)
                        ProcLockWakeup(lockMethodTable, lock);
  
  next_item:
***************
*** 1570,1576 ****
                elog(LOG, "LockReleaseAll done");
  #endif
  
!       return TRUE;
  }
  
  /*
--- 1552,1558 ----
                elog(LOG, "LockReleaseAll done");
  #endif
  
!       return;
  }
  
  /*
***************
*** 1622,1632 ****
                                        /* We want to call LockRelease just 
once */
                                        lockOwners[i].nLocks = 1;
                                        locallock->nLocks = 1;
!                                       if (!LockRelease(DEFAULT_LOCKMETHOD,
!                                                                        
&locallock->tag.lock,
!                                                                        
locallock->tag.xid,
!                                                                        
locallock->tag.mode))
!                                               elog(WARNING, 
"LockReleaseCurrentOwner: failed??");
                                }
                                break;
                        }
--- 1604,1613 ----
                                        /* We want to call LockRelease just 
once */
                                        lockOwners[i].nLocks = 1;
                                        locallock->nLocks = 1;
!                                       LockRelease(DEFAULT_LOCKMETHOD,
!                                                               
&locallock->tag.lock,
!                                                               
locallock->tag.xid,
!                                                               
locallock->tag.mode);
                                }
                                break;
                        }
Index: contrib/userlock/user_locks.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/contrib/userlock/user_locks.c,v
retrieving revision 1.15
diff -c -r1.15 user_locks.c
*** contrib/userlock/user_locks.c       29 Apr 2005 22:28:23 -0000      1.15
--- contrib/userlock/user_locks.c       13 May 2005 01:06:44 -0000
***************
*** 44,50 ****
  
        SET_LOCKTAG_USERLOCK(tag, id1, id2);
  
!       return LockRelease(USER_LOCKMETHOD, &tag, InvalidTransactionId, 
lockmode);
  }
  
  int
--- 44,52 ----
  
        SET_LOCKTAG_USERLOCK(tag, id1, id2);
  
!       LockRelease(USER_LOCKMETHOD, &tag, InvalidTransactionId, lockmode);
! 
!       return true;
  }
  
  int
***************
*** 75,79 ****
  int
  user_unlock_all(void)
  {
!       return LockReleaseAll(USER_LOCKMETHOD, true);
  }
--- 77,83 ----
  int
  user_unlock_all(void)
  {
!       LockReleaseAll(USER_LOCKMETHOD, true);
! 
!       return true;
  }
---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to