gdbusauthmechanismsha1: Don’t delete a stale lock file if it’s changed

The retry loop for acquiring the lock for the authentication cookie file
currently tries to acquire the lock for 0.5s, then gives up, assumes the
lock file is stale, and deletes it.

That’s great if the lock file *is* stale because it’s been left there by
a crashed process.

It’s not so great if the lock file just happens to have been there every
time this process checked, because the cookie file is highly contested
while (for example) running lots of parallel unit tests.

Check for that situation by comparing the mtime of the lock file and
continuing to retry if it’s changed.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #1929
This commit is contained in:
Philip Withnall 2022-02-18 13:17:19 +00:00
parent 51b707bdc6
commit dc0eb5e49a

View File

@ -38,6 +38,7 @@
#include "gdbusauthmechanismsha1.h"
#include "gcredentials.h"
#include "gdbuserror.h"
#include "glocalfileinfo.h"
#include "gioenumtypes.h"
#include "gioerror.h"
#include "gdbusprivate.h"
@ -509,6 +510,7 @@ _log (const gchar *message,
* and was created successfully) */
static gint
create_lock_exclusive (const gchar *lock_path,
gint64 *mtime_nsec,
GError **error)
{
int errsv;
@ -518,6 +520,16 @@ create_lock_exclusive (const gchar *lock_path,
errsv = errno;
if (ret < 0)
{
GLocalFileStat stat_buf;
/* Get the modification time to distinguish between the lock being stale
* or highly contested. */
if (mtime_nsec != NULL &&
g_local_file_stat (lock_path, G_LOCAL_FILE_STAT_FIELD_MTIME, G_LOCAL_FILE_STAT_FIELD_ALL, &stat_buf) == 0)
*mtime_nsec = _g_stat_mtime (&stat_buf) * G_USEC_PER_SEC * 1000 + _g_stat_mtim_nsec (&stat_buf);
else if (mtime_nsec != NULL)
*mtime_nsec = 0;
g_set_error (error,
G_IO_ERROR,
g_io_error_from_errno (errsv),
@ -538,6 +550,7 @@ keyring_acquire_lock (const gchar *path,
gint ret;
guint num_tries;
int errsv;
gint64 lock_mtime_nsec = 0, lock_mtime_nsec_prev = 0;
/* Total possible sleep period = max_tries * timeout_usec = 0.5s */
const guint max_tries = 50;
@ -565,13 +578,21 @@ keyring_acquire_lock (const gchar *path,
for (num_tries = 0; num_tries < max_tries; num_tries++)
{
lock_mtime_nsec_prev = lock_mtime_nsec;
/* Ignore the error until the final call. */
ret = create_lock_exclusive (lock, NULL);
ret = create_lock_exclusive (lock, &lock_mtime_nsec, NULL);
if (ret >= 0)
break;
/* sleep 10ms, then try again */
g_usleep (timeout_usec);
/* If the mtime of the lock file changed, dont count the retry, as it
* seems like theres contention between processes for the lock file,
* rather than a stale lock file from a crashed process. */
if (num_tries > 0 && lock_mtime_nsec != lock_mtime_nsec_prev)
num_tries--;
}
if (num_tries == max_tries)
@ -594,7 +615,7 @@ keyring_acquire_lock (const gchar *path,
_log ("Deleted stale lock file '%s'", lock);
/* Try one last time to create it, now that we've deleted the stale one */
ret = create_lock_exclusive (lock, error);
ret = create_lock_exclusive (lock, NULL, error);
if (ret < 0)
goto out;
}