# HG changeset patch # User J. Ali Harlow # Date 1528392980 -3600 # Node ID c89e5edb8eae898557351cc8a29da611ad853054 # Parent 5a49f274ab2dea9ddfb6d8eb8256c23e9dfdeadd Improve error on failure to lock database diff -r 5a49f274ab2d -r c89e5edb8eae librazor/error.c --- a/librazor/error.c Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/error.c Thu Jun 07 18:36:20 2018 +0100 @@ -42,6 +42,30 @@ return error->code; } +void +razor_error_set_object(struct razor_error *error, const char *object) +{ + if (!error) + return; + + free(error->object); + + if (object) + error->object = strdup(object); + else + error->object = NULL; + + if (error->obj_str) { + free(error->obj_str); + error->obj_str = NULL; + } + + if (error->msg) { + free(error->msg); + error->msg = NULL; + } +} + RAZOR_EXPORT const char * razor_error_get_object(struct razor_error *error) { diff -r 5a49f274ab2d -r c89e5edb8eae librazor/razor-internal.h --- a/librazor/razor-internal.h Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/razor-internal.h Thu Jun 07 18:36:20 2018 +0100 @@ -125,7 +125,7 @@ struct array file_string_pool; struct array details_string_pool; struct razor_mapped_file *mapped_files; - int lock_fd, ref_count; + int lock, ref_count; enum razor_set_flags flags; }; @@ -184,7 +184,8 @@ }; int -razor_set_acquire_lock(struct razor_set *set, const char *path, int exclusive); +razor_set_acquire_lock(struct razor_set *set, const char *path, int exclusive, + struct razor_error **error); struct razor_entry * razor_set_find_entry(struct razor_set *set, @@ -315,6 +316,7 @@ if (error) \ *(error) = razor_error_new_str(domain, code, object, str); \ else +void razor_error_set_object(struct razor_error *error, const char *object); #ifdef MSWIN_API struct razor_error *razor_error_new_mswin(const wchar_t *object, DWORD error); diff -r 5a49f274ab2d -r c89e5edb8eae librazor/razor.c --- a/librazor/razor.c Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/razor.c Thu Jun 07 18:36:20 2018 +0100 @@ -1,7 +1,7 @@ /* * Copyright (C) 2008 Kristian Høgsberg * Copyright (C) 2008 Red Hat, Inc - * Copyright (C) 2009-2012, 2016 J. Ali Harlow + * Copyright (C) 2009-2012, 2016, 2018 J. Ali Harlow * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -39,8 +39,9 @@ #include #endif +#include "razor.h" +#include "types/types.h" #include "razor-internal.h" -#include "razor.h" #ifndef O_BINARY #define O_BINARY 0 @@ -84,7 +85,7 @@ empty = array_add(&set->string_pool, 1); *empty = '\0'; - set->lock_fd = -1; + set->lock = -1; set->ref_count = 1; @@ -229,7 +230,7 @@ return NULL; } - set->lock_fd = -1; + set->lock = -1; set->ref_count = 1; if (razor_set_bind_sections(set, uri, flags, error)) { free(set); @@ -238,56 +239,189 @@ return set; } -int -razor_set_acquire_lock(struct razor_set *set, const char *uri, int exclusive) +struct razor_uri_lock { + char *uri; + int fd; + int exclusive; +}; + +static struct array razor_uri_locks = { 0, }; + +/* + * The underlying locks on MS-Windows and POSIX are different. Posix locks + * created with fcntl(F_SETLK) only affect other processes whereas MS-Windows + * locks affect the locking process as well. Partly because the latter is + * preferable and partly because it makes testing easier if platforms behave + * in the same way, we implement platform independent locks which follow the + * MS-Windows pattern. + * + * Note that razor_set_aquire_lock() currently assumes that this is a private + * helper function. Should we ever want to make this a library-wide API then + * we should probably add a 'void *owner' parameter and add a new error code + * (RAZOR_GENERAL_ERROR_LOCALLY_LOCKED?) if a lock is present with a different + * owner so that razor_set_aquire_lock() can generate a different error message. + */ + +static int +razor_uri_lock(const char *uri, int flags, mode_t mode, int exclusive, + struct razor_error **error) { - int fd; - assert(set != NULL); + int fd, i; + struct razor_uri_lock *ulock; +#ifdef MSWIN_API + DWORD lflags = LOCKFILE_FAIL_IMMEDIATELY, err; + OVERLAPPED lock = {0}; +#else + struct flock lock = {0}; +#endif - if (uri) { - fd = razor_uri_open(uri, O_CREAT | O_RDWR | O_TRUNC | O_BINARY, - 0666, NULL); - if (fd < 0) + fd = razor_uri_open(uri, flags, mode, error); + if (fd < 0) + return -1; + + for (i = ptr_array_len(&razor_uri_locks) - 1; i >= 0; i--) { + ulock = ptr_array_index(&razor_uri_locks, i); + if (ulock && !strcmp(ulock->uri, uri)) { + if (!ulock->exclusive && !exclusive) + break; + razor_set_error(error, RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_ALREADY_LOCKED, + uri, + "An existing lock is already held on this file"); + close(fd); return -1; - } else - fd = -1; + } + } #ifdef MSWIN_API - DWORD flags = LOCKFILE_FAIL_IMMEDIATELY; - OVERLAPPED lock = {0}; - if (exclusive) - flags |= LOCKFILE_EXCLUSIVE_LOCK; - if (fd >= 0 && !LockFileEx((HANDLE)_get_osfhandle(fd), flags, 0, 1, 0, - &lock)) { + lflags |= LOCKFILE_EXCLUSIVE_LOCK; + if (!LockFileEx((HANDLE)_get_osfhandle(fd), lflags, 0, 1, 0, &lock)) { + err = GetLastError(); + if (err == ERROR_IO_PENDING) + razor_set_error(error, RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_EXTERNALLY_LOCKED, + uri, + "An existing lock is already held on this file"); + else + razor_set_error_mswin(error, uri, err); close(fd); return -1; } - if (set->lock_fd >= 0) - (void)UnlockFile((HANDLE)_get_osfhandle(set->lock_fd), 0, 0, 1, - 0); #else - struct flock lock = {0}; - lock.l_type = exclusive ? F_WRLCK : F_RDLCK; lock.l_whence = SEEK_SET; lock.l_start = 0; lock.l_len = 0; - if (fd >= 0 && fcntl(fd, F_SETLK, &lock) < 0) { + if (fcntl(fd, F_SETLK, &lock) < 0) { + if (errno == EAGAIN || errno == EACCES) + razor_set_error(error, RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_EXTERNALLY_LOCKED, + uri, + "An existing lock is already held on this file"); + else + razor_set_error_posix(error, uri); close(fd); return -1; } - if (set->lock_fd >= 0) { - lock.l_type = F_UNLCK; - (void)fcntl(set->lock_fd, F_SETLK, &lock); - } #endif - if (set->lock_fd >= 0) - close(set->lock_fd); - set->lock_fd = fd; + ulock = zalloc(sizeof *ulock); + ulock->uri = strdup(uri); + ulock->fd = fd; + ulock->exclusive = exclusive; + return ptr_array_add(&razor_uri_locks, ulock); +} - return 0; +static void +razor_uri_unlock(int id) +{ + struct razor_uri_lock *ulock; +#ifndef MSWIN_API + struct flock lock = {0}; +#endif + + if (id < 0 || id >= ptr_array_len(&razor_uri_locks)) + return; + + ulock = ptr_array_index(&razor_uri_locks, id); + + if (!ulock) + return; + +#ifdef MSWIN_API + (void)UnlockFile((HANDLE)_get_osfhandle(ulock->fd), 0, 0, 1, 0); +#else + lock.l_type = F_UNLCK; + lock.l_whence = SEEK_SET; + lock.l_start = 0; + lock.l_len = 0; + (void)fcntl(ulock->fd, F_SETLK, &lock); +#endif + + ptr_array_remove_index(&razor_uri_locks, id); + free(ulock->uri); + close(ulock->fd); + free(ulock); +} + +int +razor_set_acquire_lock(struct razor_set *set, const char *uri, int exclusive, + struct razor_error **error) +{ + int lock; + const char *errstr = NULL; + assert(set != NULL); + + if (!uri) { + razor_uri_unlock(set->lock); + set->lock = -1; + return 0; + } + + lock = razor_uri_lock(uri, + O_CREAT | O_RDWR | O_TRUNC | O_BINARY, + 0666, exclusive, error); + + if (lock >= 0) { + set->lock = lock; + return 0; + } + + if (!error) + return -1; + + if (razor_error_matches(*error, RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_ALREADY_LOCKED)) { + /* + * This represents a design flaw in the caller. + */ + if (exclusive) + errstr = "Database is in use and cannot be locked"; + else + errstr = "Database is being updated and cannot be locked"; + } else if (razor_error_matches(*error, RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_EXTERNALLY_LOCKED)) { + /* + * This is probably caused by another application holding + * the lock, but under MS-Windows the file could in theory + * be locked by this application outside of razor_set. + */ + if (exclusive) + errstr = "Database is in use. Please try again later"; + else + errstr = "Database is being updated. Please try again later"; + } + + if (errstr) { + razor_error_free(*error); + razor_set_error(error, + RAZOR_GENERAL_ERROR, + RAZOR_GENERAL_ERROR_DATABASE_LOCKED, + uri, errstr); + } + + return -1; } static void @@ -312,7 +446,7 @@ } } - razor_set_acquire_lock(set, NULL, 0); + razor_set_acquire_lock(set, NULL, 0, NULL); free(set); } diff -r 5a49f274ab2d -r c89e5edb8eae librazor/razor.h.in --- a/librazor/razor.h.in Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/razor.h.in Thu Jun 07 18:36:20 2018 +0100 @@ -1,7 +1,8 @@ /* * Copyright (C) 2008 Kristian Høgsberg * Copyright (C) 2008 Red Hat, Inc - * Copyright (C) 2009, 2011, 2012, 2014, 2016 J. Ali Harlow + * Copyright (C) 2009, 2011, 2012, 2014, 2016, 2018 + * J. Ali Harlow * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -152,6 +153,8 @@ RAZOR_GENERAL_ERROR_UNSUPPORTED_URI, RAZOR_GENERAL_ERROR_BAD_URI, RAZOR_GENERAL_ERROR_UNSUPPORTED_ARCHIVE, + RAZOR_GENERAL_ERROR_ALREADY_LOCKED, + RAZOR_GENERAL_ERROR_EXTERNALLY_LOCKED, }; int razor_error_get_domain(struct razor_error *error); diff -r 5a49f274ab2d -r c89e5edb8eae librazor/root.c --- a/librazor/root.c Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/root.c Thu Jun 07 18:36:20 2018 +0100 @@ -1,7 +1,7 @@ /* * Copyright (C) 2008 Kristian Høgsberg * Copyright (C) 2008 Red Hat, Inc - * Copyright (C) 2009, 2011, 2012, 2014, 2016 J. Ali Harlow + * Copyright (C) 2009, 2011, 2012, 2014, 2016, 2018 J. Ali Harlow * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -67,6 +67,7 @@ #endif static char *razor_database_uri = RAZOR_DATABASE_URI; static int razor_database_uri_alloced = FALSE; +static int razor_database_uri_default = TRUE; struct razor_root { struct razor_set *system; @@ -107,9 +108,11 @@ if (database_uri) { razor_database_uri = strdup(database_uri); razor_database_uri_alloced = TRUE; + razor_database_uri_default = FALSE; } else { razor_database_uri = RAZOR_DATABASE_URI; razor_database_uri_alloced = FALSE; + razor_database_uri_default = TRUE; } } @@ -223,14 +226,13 @@ return NULL; } - r = razor_set_acquire_lock(image->system, lock_uri, 1); + r = razor_set_acquire_lock(image->system, lock_uri, 1, error); free(lock_uri); if (r < 0) { - razor_set_error(error, RAZOR_GENERAL_ERROR, - RAZOR_GENERAL_ERROR_DATABASE_LOCKED, NULL, - "Failed to acquire exclusive system lock"); + if (error && razor_database_uri_default) + razor_error_set_object(*error, root_uri); razor_set_unref(image->system); free(image); return NULL; @@ -277,14 +279,13 @@ return NULL; } - r = razor_set_acquire_lock(set, uri, 0); + r = razor_set_acquire_lock(set, uri, 0, error); free(uri); if (r < 0) { - razor_set_error(error, RAZOR_GENERAL_ERROR, - RAZOR_GENERAL_ERROR_DATABASE_LOCKED, NULL, - "Failed to acquire non-exclusive system lock"); + if (error && razor_database_uri_default) + razor_error_set_object(*error, root_uri); razor_set_unref(set); return NULL; } diff -r 5a49f274ab2d -r c89e5edb8eae librazor/types/array.c --- a/librazor/types/array.c Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/types/array.c Thu Jun 07 18:36:20 2018 +0100 @@ -1,7 +1,7 @@ /* * Copyright (C) 2008 Kristian Høgsberg * Copyright (C) 2008 Red Hat, Inc - * Copyright (C) 2016 J. Ali Harlow + * Copyright (C) 2016, 2018 J. Ali Harlow * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -74,7 +74,16 @@ return array->data + array->size - size; } -void ptr_array_add(struct array *ptr_array, void *data) +int ptr_array_len(struct array *ptr_array) +{ + void **ptrend; + + ptrend = ptr_array->data + ptr_array->size; + + return ptrend - (void **)ptr_array->data; +} + +int ptr_array_add(struct array *ptr_array, void *data) { void **ptr, **ptrend; @@ -87,6 +96,21 @@ ptr = array_add(ptr_array, sizeof *ptr); *ptr = data; + + return ptr - (void **)ptr_array->data; +} + +void *ptr_array_index(struct array *ptr_array, int indx) +{ + void **ptr, **ptrend; + + ptrend = ptr_array->data + ptr_array->size; + + ptr = (void **)ptr_array->data + indx; + if (ptr < ptr_array->data || ptr >= ptrend) + return NULL; + + return *ptr; } int ptr_array_find(struct array *ptr_array, void *data) @@ -108,7 +132,7 @@ ptrend = ptr_array->data + ptr_array->size; ptr = (void **)ptr_array->data + indx; - if (ptr >= ptrend) + if (ptr < ptr_array->data || ptr >= ptrend) return; *ptr = NULL; diff -r 5a49f274ab2d -r c89e5edb8eae librazor/types/types.h --- a/librazor/types/types.h Tue Jun 05 11:07:53 2018 +0100 +++ b/librazor/types/types.h Thu Jun 07 18:36:20 2018 +0100 @@ -1,7 +1,7 @@ /* * Copyright (C) 2008 Kristian Høgsberg * Copyright (C) 2008 Red Hat, Inc - * Copyright (C) 2009 J. Ali Harlow + * Copyright (C) 2009, 2018 J. Ali Harlow * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -37,7 +37,9 @@ void array_release(struct array *array); int array_set_size(struct array *array, int size); void *array_add(struct array *array, int size); -void ptr_array_add(struct array *ptr_array, void *data); +int ptr_array_len(struct array *ptr_array); +int ptr_array_add(struct array *ptr_array, void *data); +void *ptr_array_index(struct array *ptr_array, int indx); int ptr_array_find(struct array *ptr_array, void *data); void ptr_array_remove_index(struct array *ptr_array, int indx);