gvfs r2056 - in trunk: . client
- From: hansp svn gnome org
- To: svn-commits-list gnome org
- Subject: gvfs r2056 - in trunk: . client
- Date: Fri, 17 Oct 2008 06:55:20 +0000 (UTC)
Author: hansp
Date: Fri Oct 17 06:55:20 2008
New Revision: 2056
URL: http://svn.gnome.org/viewvc/gvfs?rev=2056&view=rev
Log:
2008-10-17 Hans Petter Jansson <hpj novell com>
Attempt to prevent potential race conditions in the FUSE backend
when file handles get closed while still in use in another thread,
if that ever happens.
* client/gvfsfusedaemon.c (file_handle_new): Insert new file
handles in global hash table of active file handles.
(file_handle_unref): Clarify the code and comments a little.
(file_handle_free): Remove file handle from global table of
active handles.
(reindex_file_handle_for_path)
(get_file_handle_for_path)
(get_or_create_file_handle_for_path): global_fh_table ->
global_path_to_fh_map.
(get_file_handle_from_info): New function that recovers our file
handle from a fuse_file_info struct, but only if it exists in
the global table of valid handles.
(vfs_getattr): Remove code that acquired and locked the file handle
for the path we operate on. No locking is required here.
(vfs_open): Assign file handle to fuse_file_info while holding lock.
Purely a formality that makes code easier to read.
(vfs_create): Ditto.
(vfs_release): Use get_file_handle_from_info () so the file handle
is validated.
(vfs_read): Hold a ref to the file handle while it's in use. If
handle is invalid, raise EINVAL.
(vfs_ftruncate): Ditto.
(vfs_write): Ditto.
(vfs_rename): Cosmetic change.
(vfs_unlink): Ditto.
(vfs_truncate): Add helpful comment.
(vfs_init): Create global table of active file handles.
Modified:
trunk/ChangeLog
trunk/client/gvfsfusedaemon.c
Modified: trunk/client/gvfsfusedaemon.c
==============================================================================
--- trunk/client/gvfsfusedaemon.c (original)
+++ trunk/client/gvfsfusedaemon.c Fri Oct 17 06:55:20 2008
@@ -79,22 +79,23 @@
size_t pos;
} FileHandle;
-static GThread *subthread = NULL;
-static GMainLoop *subthread_main_loop = NULL;
-static GVfs *gvfs = NULL;
+static GThread *subthread = NULL;
+static GMainLoop *subthread_main_loop = NULL;
+static GVfs *gvfs = NULL;
-static GVolumeMonitor *volume_monitor = NULL;
+static GVolumeMonitor *volume_monitor = NULL;
/* Contains pointers to MountRecord */
-static GList *mount_list = NULL;
-static GMutex *mount_list_mutex;
+static GList *mount_list = NULL;
+static GMutex *mount_list_mutex;
-static time_t daemon_creation_time;
-static uid_t daemon_uid;
-static gid_t daemon_gid;
-
-static GStaticMutex global_mutex = G_STATIC_MUTEX_INIT;
-static GHashTable *global_fh_table = NULL;
+static time_t daemon_creation_time;
+static uid_t daemon_uid;
+static gid_t daemon_gid;
+
+static GStaticMutex global_mutex = G_STATIC_MUTEX_INIT;
+static GHashTable *global_path_to_fh_map = NULL;
+static GHashTable *global_active_fh_map = NULL;
/* ------- *
* Helpers *
@@ -194,6 +195,8 @@
file_handle->op = FILE_OP_NONE;
file_handle->path = g_strdup (path);
+ g_hash_table_insert (global_active_fh_map, file_handle, file_handle);
+
return file_handle;
}
@@ -209,14 +212,18 @@
{
if (g_atomic_int_dec_and_test (&file_handle->refcount))
{
+ gint refs;
+
g_static_mutex_lock (&global_mutex);
/* Test again, since e.g. get_file_handle_for_path() might have
* snatched the global mutex and revived the file handle between
- * g_atomic_int_dec_and_test() and us obtaining the lock. */
+ * g_atomic_int_dec_and_test() and us obtaining the global lock. */
+
+ refs = g_atomic_int_get (&file_handle->refcount);
- if (g_atomic_int_get (&file_handle->refcount) == 0)
- g_hash_table_remove (global_fh_table, file_handle->path);
+ if (refs == 0)
+ g_hash_table_remove (global_path_to_fh_map, file_handle->path);
g_static_mutex_unlock (&global_mutex);
}
@@ -252,6 +259,8 @@
static void
file_handle_free (FileHandle *file_handle)
{
+ g_hash_table_remove (global_active_fh_map, file_handle);
+
file_handle_close_stream (file_handle);
g_mutex_free (file_handle->mutex);
g_free (file_handle->path);
@@ -265,12 +274,12 @@
g_static_mutex_lock (&global_mutex);
- fh = g_hash_table_lookup (global_fh_table, path);
+ fh = g_hash_table_lookup (global_path_to_fh_map, path);
+
if (fh)
file_handle_ref (fh);
g_static_mutex_unlock (&global_mutex);
-
return fh;
}
@@ -281,7 +290,7 @@
g_static_mutex_lock (&global_mutex);
- fh = g_hash_table_lookup (global_fh_table, path);
+ fh = g_hash_table_lookup (global_path_to_fh_map, path);
if (fh)
{
@@ -290,11 +299,30 @@
else
{
fh = file_handle_new (path);
- g_hash_table_insert (global_fh_table, fh->path, fh);
+ g_hash_table_insert (global_path_to_fh_map, fh->path, fh);
}
g_static_mutex_unlock (&global_mutex);
+ return fh;
+}
+
+static FileHandle *
+get_file_handle_from_info (struct fuse_file_info *fi)
+{
+ FileHandle *fh;
+ g_static_mutex_lock (&global_mutex);
+
+ fh = GET_FILE_HANDLE (fi);
+
+ /* If the file handle is still valid, its value won't change. If
+ * invalid, it's set to NULL. */
+ fh = g_hash_table_lookup (global_active_fh_map, fh);
+
+ if (fh)
+ file_handle_ref (fh);
+
+ g_static_mutex_unlock (&global_mutex);
return fh;
}
@@ -306,17 +334,17 @@
g_static_mutex_lock (&global_mutex);
- if (!g_hash_table_lookup_extended (global_fh_table, old_path,
+ if (!g_hash_table_lookup_extended (global_path_to_fh_map, old_path,
(gpointer *) &old_path_internal,
(gpointer *) &fh))
goto out;
- g_hash_table_steal (global_fh_table, old_path);
+ g_hash_table_steal (global_path_to_fh_map, old_path);
g_free (fh->path);
fh->path = g_strdup (new_path);
- g_hash_table_insert (global_fh_table, fh->path, fh);
+ g_hash_table_insert (global_path_to_fh_map, fh->path, fh);
out:
g_static_mutex_unlock (&global_mutex);
@@ -779,14 +807,9 @@
{
GFile *file;
gint result = 0;
- FileHandle *fh;
debug_print ("vfs_getattr: %s\n", path);
- fh = get_file_handle_for_path (path);
- if (fh)
- g_mutex_lock (fh->mutex);
-
memset (sbuf, 0, sizeof (*sbuf));
sbuf->st_dev = 0; /* dev_t ID of device containing file */
@@ -825,12 +848,6 @@
result = -ENOENT;
}
- if (fh)
- {
- g_mutex_unlock (fh->mutex);
- file_handle_unref (fh);
- }
-
debug_print ("vfs_getattr: -> %s\n", strerror (-result));
return result;
@@ -954,7 +971,9 @@
if (file_type == G_FILE_TYPE_REGULAR)
{
FileHandle *fh = get_or_create_file_handle_for_path (path);
-
+
+ g_mutex_lock (fh->mutex);
+
/* File exists */
SET_FILE_HANDLE (fi, fh);
@@ -963,8 +982,6 @@
/* Set up a stream here, so we can check for errors */
- g_mutex_lock (fh->mutex);
-
if (fi->flags & O_WRONLY || fi->flags & O_RDWR)
result = setup_output_stream (file, fh);
else
@@ -1053,10 +1070,10 @@
/* Success */
- SET_FILE_HANDLE (fi, fh);
-
g_mutex_lock (fh->mutex);
+ SET_FILE_HANDLE (fi, fh);
+
file_handle_close_stream (fh);
fh->stream = file_output_stream;
fh->op = FILE_OP_WRITE;
@@ -1087,12 +1104,16 @@
static gint
vfs_release (const gchar *path, struct fuse_file_info *fi)
{
- FileHandle *fh = GET_FILE_HANDLE (fi);
+ FileHandle *fh = get_file_handle_from_info (fi);
debug_print ("vfs_release: %s\n", path);
if (fh)
- file_handle_unref (fh);
+ {
+ /* get_file_handle_from_info () adds a "working ref", so unref twice. */
+ file_handle_unref (fh);
+ file_handle_unref (fh);
+ }
return 0;
}
@@ -1209,23 +1230,32 @@
if ((file = file_from_full_path (path)))
{
- FileHandle *fh = GET_FILE_HANDLE (fi);
+ FileHandle *fh = get_file_handle_from_info (fi);
- g_mutex_lock (fh->mutex);
+ if (fh)
+ {
+ g_mutex_lock (fh->mutex);
- result = setup_input_stream (file, fh);
+ result = setup_input_stream (file, fh);
- if (result == 0)
- {
- result = read_stream (fh, buf, size, offset);
+ if (result == 0)
+ {
+ result = read_stream (fh, buf, size, offset);
+ }
+ else
+ {
+ debug_print ("vfs_read: failed to setup input_stream!\n");
+ }
+
+ g_mutex_unlock (fh->mutex);
+ file_handle_unref (fh);
}
else
{
- debug_print ("vfs_read: failed to setup input_stream!\n");
+ result = -EINVAL;
}
g_object_unref (file);
- g_mutex_unlock (fh->mutex);
}
else
{
@@ -1328,18 +1358,27 @@
if ((file = file_from_full_path (path)))
{
- FileHandle *fh = GET_FILE_HANDLE (fi);
+ FileHandle *fh = get_file_handle_from_info (fi);
- g_mutex_lock (fh->mutex);
+ if (fh)
+ {
+ g_mutex_lock (fh->mutex);
- result = setup_output_stream (file, fh);
- if (result == 0)
+ result = setup_output_stream (file, fh);
+ if (result == 0)
+ {
+ result = write_stream (fh, buf, len, offset);
+ }
+
+ g_mutex_unlock (fh->mutex);
+ file_handle_unref (fh);
+ }
+ else
{
- result = write_stream (fh, buf, len, offset);
+ result = -EINVAL;
}
g_object_unref (file);
- g_mutex_unlock (fh->mutex);
}
else
{
@@ -1497,9 +1536,7 @@
if (old_file && new_file)
{
- FileHandle *fh;
-
- fh = get_file_handle_for_path (old_path);
+ FileHandle *fh = get_file_handle_for_path (old_path);
if (fh)
{
@@ -1561,9 +1598,7 @@
if (file)
{
- FileHandle *fh;
-
- fh = get_file_handle_for_path (path);
+ FileHandle *fh = get_file_handle_for_path (path);
if (fh)
{
@@ -1706,40 +1741,49 @@
if (file)
{
- FileHandle *fh = GET_FILE_HANDLE (fi);
+ FileHandle *fh = get_file_handle_from_info (fi);
- g_mutex_lock (fh->mutex);
+ if (fh)
+ {
+ g_mutex_lock (fh->mutex);
- result = setup_output_stream (file, fh);
+ result = setup_output_stream (file, fh);
- if (result == 0)
- {
- if (g_seekable_can_truncate (G_SEEKABLE (fh->stream)))
- {
- g_seekable_truncate (fh->stream, size, NULL, &error);
- }
- else if (size == 0)
+ if (result == 0)
{
- g_output_stream_close (fh->stream, NULL, NULL);
- g_object_unref (fh->stream);
- fh->stream = NULL;
+ if (g_seekable_can_truncate (G_SEEKABLE (fh->stream)))
+ {
+ g_seekable_truncate (fh->stream, size, NULL, &error);
+ }
+ else if (size == 0)
+ {
+ g_output_stream_close (fh->stream, NULL, NULL);
+ g_object_unref (fh->stream);
+ fh->stream = NULL;
- fh->stream = g_file_replace (file, 0, FALSE, 0, NULL, &error);
- }
- else
- {
- result = -ENOTSUP;
- }
+ fh->stream = g_file_replace (file, 0, FALSE, 0, NULL, &error);
+ }
+ else
+ {
+ result = -ENOTSUP;
+ }
- if (error)
- {
- result = -errno_from_error (error);
- g_error_free (error);
+ if (error)
+ {
+ result = -errno_from_error (error);
+ g_error_free (error);
+ }
}
+
+ g_mutex_unlock (fh->mutex);
+ file_handle_unref (fh);
+ }
+ else
+ {
+ result = -EINVAL;
}
g_object_unref (file);
- g_mutex_unlock (fh->mutex);
}
else
{
@@ -1767,6 +1811,7 @@
GFileOutputStream *file_output_stream = NULL;
FileHandle *fh;
+ /* Get a file handle just to lock the path while we're working */
fh = get_file_handle_for_path (path);
if (fh)
g_mutex_lock (fh->mutex);
@@ -2105,8 +2150,10 @@
daemon_gid = getgid ();
mount_list_mutex = g_mutex_new ();
- global_fh_table = g_hash_table_new_full (g_str_hash, g_str_equal,
- NULL, (GDestroyNotify) file_handle_free);
+ global_path_to_fh_map = g_hash_table_new_full (g_str_hash, g_str_equal,
+ NULL, (GDestroyNotify) file_handle_free);
+ global_active_fh_map = g_hash_table_new_full (g_direct_hash, g_direct_equal,
+ NULL, NULL);
dbus_error_init (&error);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
