Maniphest T8317

Recursive trustdb lock problem
Testing, NormalPublic
Actions

Assigned To
• gniibe
Authored By
• gniibe
Thu, Jul 2, 4:50 AM
Tags
Subscribers
• gniibe
pl13

Description

Recursive lock is supported by the commit: rG456a3a8e93ea: gpg: Fix trustdb updates without lock held.
for T3839

But it was actually supported in a different way by the take_write_lock which was introduced by the commit: rGfe5c6edaed78: g10: Fix a race condition initially creating trustdb.
for T1675

Before the is_locked is introduced, it was the caller which takes care of recursive locking; It remains the code in tdbio_sync, where it checks the return value of take_write_lock (and remember with did_lock), and only call release_write_lock when did_lock is true.

This did_lock thing should be removed now. Currently, when recursive lock happens in tdbio_sync, the lock remains (until cleanup by exit).

Revisions and Commits

rG GnuPG
rG7752c7dcad89 gpg: Fix trustdb recursive lock problem.

Related Objects
Search...

Event Timeline

• gniibe triaged this task as Normal priority.Thu, Jul 2, 4:50 AM
• gniibe created this task.

I mean:

diff --git a/g10/tdbio.c b/g10/tdbio.c
index 7ee62fca0..ca7e94782 100644
--- a/g10/tdbio.c
+++ b/g10/tdbio.c
@@ -428,7 +428,6 @@ int
 tdbio_sync (void)
 {
     CACHE_CTRL r;
-    int did_lock = 0;
 
     if( db_fd == -1 )
 	open_db();
@@ -440,9 +439,7 @@ tdbio_sync (void)
     if( !cache_is_dirty )
 	return 0;
 
-    if (!take_write_lock ())
-        did_lock = 1;
-
+    take_write_lock ();
     for( r = cache_list; r; r = r->next ) {
 	if( r->flags.used && r->flags.dirty ) {
 	    int rc = write_cache_item( r );
@@ -451,8 +448,7 @@ tdbio_sync (void)
 	}
     }
     cache_is_dirty = 0;
-    if (did_lock)
-        release_write_lock ();
+    release_write_lock ();
 
     return 0;
 }
• gniibe added a parent task: T1675: gpg --verify has race conditions when used concurrently.Fri, Jul 3, 1:48 AM
• gniibe added a commit: rG7752c7dcad89: gpg: Fix trustdb recursive lock problem..Fri, Jul 3, 3:46 AM
• gniibe changed the task status from Open to Testing.Fri, Jul 3, 3:55 AM

Apply the fix in: rG7752c7dcad89: gpg: Fix trustdb recursive lock problem.

Note that this fix won't improve the situation of T8305.

pl13 added a subscriber: pl13.Fri, Jul 3, 8:34 AM