Code paths where LWLock should be released on failure

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Code paths where LWLock should be released on failure
Date: 2015-04-23 04:51:57
Message-ID: CAB7nPqQOtvH7gbJHc8CF_cthSEuoSAuEeuyTXVKLPfeV9_jOPQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

After looking at bug #13128, I have been looking at the code around
LWLockAcquire/Release to see if there are similar issues elsewhere.
Here are my findings:

1) SimpleLruReadPage() holds a control lock at entry that will be held
at exit as well. However SlruReportIOError() can report an error,
letting the lock hold. Shouldn't we release the control lock when a
failure happens?

2) The patch attached to #13128 fixes MarkAsPreparing(), but actually
twophase.c also does not release some locks in LockGXact().

3) PreCommit_Notify(at)async(dot)c should release AsyncQueueLock on failure I
guess because it is called at transaction commit. At least it looks
safer this way.

4) TablespaceCreateDbspace does not release TablespaceCreateLock but
LWLockReleaseAll would do it when aborting its transaction, so no
changes done there (?).

5) In ReplicationSlotCreate(at)slot(dot)c, I would think that
ReplicationSlotAllocationLock should be released when all the locks
are in use. Similarly, in ReplicationSlotDropAcquired,
ReplicationSlotAllocationLock should be released when !fail_softly.
SaveSlotToPath could also be made safer when a file could not be
created.

6) In dsm.c, dsm_create does not release
DynamicSharedMemoryControlLock when Error'ing when there are too many
shared memory segments.

7) In shmem.c, ShmemInitStruct does not release ShmemIndexLock on OOM.
I guess that's fine in bootstrap mode, still...

8) In lock.c, LockRelease() does not release partitionLock when a
shared lock cannot be found. Similar thing with
FastPathGetRelationLockEntry().

9) In predicate.c, CreatePredicateLock() forgets to release
SerializablePredicateLockListLock and partitionLock in case of an OOM.
There is a similar issue with ReleaseOneSerializableXact(),
CheckForSerializableConflictOut() and
predicatelock_twophase_recover().

10) In relcache.c, RelCacheInitLock is not released in
RelationCacheInitFilePreInvalidate() after unlink() failure.

11) In AlterSystemSetConfigFile(), AutoFileLock should be released on failure.

All those things give the patch attached. Comments are welcome.
Regards,
--
Michael

Attachment Content-Type Size
20150423_lwlocks_failure_release.patch text/x-patch 11.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-04-23 05:46:16 Re: Code paths where LWLock should be released on failure
Previous Message David G. Johnston 2015-04-23 03:35:04 Re: [BUGS] Failure to coerce unknown type to specific type