Merge branch '1929-dbus-auth-locking' into 'main'

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

Closes #1929

See merge request GNOME/glib!2511
This commit is contained in:
Simon McVittie 2022-02-21 13:48:54 +00:00
commit 3c12ddce81
3 changed files with 44 additions and 22 deletions

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;
}

View File

@ -317,8 +317,12 @@ inline static blkcnt_t _g_stat_blocks (const GLocalFileStat *buf) { return b
inline static time_t _g_stat_atime (const GLocalFileStat *buf) { return buf->st_atime; }
inline static time_t _g_stat_ctime (const GLocalFileStat *buf) { return buf->st_ctime; }
inline static time_t _g_stat_mtime (const GLocalFileStat *buf) { return buf->st_mtime; }
#else
inline static time_t _g_stat_atime (const GLocalFileStat *buf) { return buf->st_atim.tv_sec; }
inline static time_t _g_stat_ctime (const GLocalFileStat *buf) { return buf->st_ctim.tv_sec; }
inline static time_t _g_stat_mtime (const GLocalFileStat *buf) { return buf->st_mtim.tv_sec; }
#endif
#ifdef HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC
#if defined(HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC) || defined(G_OS_WIN32)
inline static guint32 _g_stat_atim_nsec (const GLocalFileStat *buf) { return buf->st_atim.tv_nsec; }
inline static guint32 _g_stat_ctim_nsec (const GLocalFileStat *buf) { return buf->st_ctim.tv_nsec; }
inline static guint32 _g_stat_mtim_nsec (const GLocalFileStat *buf) { return buf->st_mtim.tv_nsec; }

View File

@ -179,8 +179,8 @@ setup_client_cb (GObject *source,
f->client_conn = g_dbus_connection_new_finish (res, &f->error);
g_assert_no_error (f->error);
g_assert (G_IS_DBUS_CONNECTION (f->client_conn));
g_assert (f->client_conn == G_DBUS_CONNECTION (source));
g_assert_true (G_IS_DBUS_CONNECTION (f->client_conn));
g_assert_true (f->client_conn == G_DBUS_CONNECTION (source));
}
static void
@ -192,8 +192,8 @@ setup_server_cb (GObject *source,
f->server_conn = g_dbus_connection_new_finish (res, &f->error);
g_assert_no_error (f->error);
g_assert (G_IS_DBUS_CONNECTION (f->server_conn));
g_assert (f->server_conn == G_DBUS_CONNECTION (source));
g_assert_true (G_IS_DBUS_CONNECTION (f->server_conn));
g_assert_true (f->server_conn == G_DBUS_CONNECTION (source));
}
static void
@ -206,21 +206,21 @@ setup (Fixture *f,
ok = test_pipe (&f->server_istream, &f->client_real_ostream, &f->error);
g_assert_no_error (f->error);
g_assert (G_IS_OUTPUT_STREAM (f->client_real_ostream));
g_assert (G_IS_INPUT_STREAM (f->server_istream));
g_assert (ok);
g_assert_true (G_IS_OUTPUT_STREAM (f->client_real_ostream));
g_assert_true (G_IS_INPUT_STREAM (f->server_istream));
g_assert_true (ok);
f->client_ostream = g_object_new (MY_TYPE_OUTPUT_STREAM,
"base-stream", f->client_real_ostream,
"close-base-stream", TRUE,
NULL);
g_assert (G_IS_OUTPUT_STREAM (f->client_ostream));
g_assert_true (G_IS_OUTPUT_STREAM (f->client_ostream));
ok = test_pipe (&f->client_istream, &f->server_ostream, &f->error);
g_assert_no_error (f->error);
g_assert (G_IS_OUTPUT_STREAM (f->server_ostream));
g_assert (G_IS_INPUT_STREAM (f->client_istream));
g_assert (ok);
g_assert_true (G_IS_OUTPUT_STREAM (f->server_ostream));
g_assert_true (G_IS_INPUT_STREAM (f->client_istream));
g_assert_true (ok);
f->client_stream = test_io_stream_new (f->client_istream, f->client_ostream);
f->server_stream = test_io_stream_new (f->server_istream, f->server_ostream);
@ -244,13 +244,13 @@ flush_cb (GObject *source,
Fixture *f = user_data;
gboolean ok;
g_assert (G_IS_DBUS_CONNECTION (source));
g_assert (G_IS_DBUS_CONNECTION (f->client_conn));
g_assert_true (G_IS_DBUS_CONNECTION (source));
g_assert_true (G_IS_DBUS_CONNECTION (f->client_conn));
g_assert_cmpuint ((guintptr) f->client_conn, ==, (guintptr) G_DBUS_CONNECTION (source));
ok = g_dbus_connection_flush_finish (f->client_conn, res, &f->error);
g_assert_no_error (f->error);
g_assert (ok);
g_assert_true (ok);
f->flushed = TRUE;
}
@ -270,7 +270,7 @@ test_flush_busy (Fixture *f,
"com.example.Foo", "SomeSignal", NULL,
&f->error);
g_assert_no_error (f->error);
g_assert (ok);
g_assert_true (ok);
/* wait for at least part of the message to have started writing -
* the write will block indefinitely in the worker thread
@ -318,7 +318,7 @@ test_flush_idle (Fixture *f,
"com.example.Foo", "SomeSignal", NULL,
&f->error);
g_assert_no_error (f->error);
g_assert (ok);
g_assert_true (ok);
/* wait for at least part of the message to have been written */
do {
@ -368,9 +368,6 @@ main (int argc,
{
gint ret;
/* FIXME: Add debug for https://gitlab.gnome.org/GNOME/glib/issues/1929 */
g_setenv ("G_DBUS_DEBUG", "authentication", TRUE);
g_test_init (&argc, &argv, NULL);
g_test_add ("/gdbus/connection/flush/busy", Fixture, NULL,