[gamin] gamin-lose-watch-fix.patch
- From: John McCutchan <ttb tentacle dhs org>
- To: gamin-list gnome org
- Subject: [gamin] gamin-lose-watch-fix.patch
- Date: Mon, 25 Jul 2005 18:52:17 -0400
Yo,
I'm attaching a patch that fixes the reference counting bugs
http://bugzilla.gnome.org/show_bug.cgi?id=172365 ,
http://bugzilla.gnome.org/show_bug.cgi?id=302236 , and countless distro
specific bug reports.
The bug symptom was that the kernel backends would drop the refcount to
0 and then stop watching a directory, while the subscription layer would
think that it was still watching the directory.
The cause of the bug was a missing return statement when removing a
subscription that was on the new subscription list. Gamin would ask the
kernel to stop watching a directory when it had never asked it to watch
it in the first place (because it was still on the new queue). This
caused the kernel ref counting to screw up. This was also why that
assert was causing a crash for me.
The patch also includes some debug changes, and I factored out the get
pid name code. I'm away from my main development machine, so I won't be
able to commit this until Sunday. Feel free to commit before then.
John McCutchan
Index: server/Makefile.am
===================================================================
RCS file: /cvs/gnome/gamin/server/Makefile.am,v
retrieving revision 1.16
diff -u -r1.16 Makefile.am
--- server/Makefile.am 8 Jun 2005 21:48:00 -0000 1.16
+++ server/Makefile.am 25 Jul 2005 22:34:34 -0000
@@ -29,6 +29,8 @@
gam_tree.h \
gam_poll.c \
gam_poll.h \
+ gam_pidname.c \
+ gam_pidname.h \
gam_channel.c \
gam_channel.h \
gam_connection.c \
@@ -38,6 +40,7 @@
gam_excludes.c \
gam_excludes.h \
local_inotify.h \
+ local_inotify_syscalls.h \
gam_debug_lists.c \
server_config.h
Index: server/gam_connection.c
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_connection.c,v
retrieving revision 1.21
diff -u -r1.21 gam_connection.c
--- server/gam_connection.c 21 Jul 2005 18:17:06 -0000 1.21
+++ server/gam_connection.c 25 Jul 2005 22:34:34 -0000
@@ -10,6 +10,7 @@
#include "gam_protocol.h"
#include "gam_channel.h"
#include "gam_error.h"
+#include "gam_pidname.h"
#ifdef GAMIN_DEBUG_API
#include "gam_debugging.h"
#endif
@@ -35,6 +36,24 @@
GamListener *listener; /* the listener associated with the connection */
};
+static const char *
+gam_reqtype_to_string (GAMReqType type)
+{
+ switch (type)
+ {
+ case GAM_REQ_FILE:
+ return "MONFILE";
+ case GAM_REQ_DIR:
+ return "MONDIR";
+ case GAM_REQ_CANCEL:
+ return "CANCEL";
+ case GAM_REQ_DEBUG:
+ return "4";
+ }
+
+ return "";
+}
+
/**
* gam_connections_init:
*
@@ -200,10 +219,6 @@
gam_connection_set_pid(GamConnDataPtr conn, int pid)
{
g_assert(conn);
-#ifdef HAVE_LINUX
- gchar *procname;
- FILE *fp;
-#endif
if (conn->state != GAM_STATE_AUTH) {
GAM_DEBUG(DEBUG_INFO, "Connection in unexpected state: "
@@ -214,34 +229,7 @@
conn->state = GAM_STATE_OKAY;
conn->pid = pid;
-#ifdef HAVE_LINUX
- procname = g_strdup_printf ("/proc/%d/cmdline", pid);
- fp = fopen(procname, "r");
- g_free (procname);
- if (!fp) {
- conn->pidname = g_strdup_printf ("%d", pid);
- } else {
- gchar *name = g_malloc (128);
- int i = 0;
- while (i < 128) {
- int ch = fgetc (fp);
-
- if (ch == EOF)
- break;
-
- name[i++] = ch;
-
- if (ch == '\0')
- break;
- }
- name[127] = '\0';
- conn->pidname = g_strdup (name);
- g_free (name);
- fclose (fp);
- }
-#else
- conn->pidname = g_strdup_printf ("%d", pid);
-#endif
+ conn->pidname = gam_get_pidname (pid);
conn->listener = gam_listener_new(conn, pid);
if (conn->listener == NULL) {
GAM_DEBUG(DEBUG_INFO, "Failed to create listener\n");
@@ -317,8 +305,8 @@
type = req->type & 0xF;
options = req->type & 0xFFF0;
- GAM_DEBUG(DEBUG_INFO, "Request: from %s, seq %d, type %x options %x\n",
- conn->pidname, req->seq, type, options);
+ GAM_DEBUG(DEBUG_INFO, "%s request: from %s, seq %d, type %x options %x\n",
+ gam_reqtype_to_string (type), conn->pidname, req->seq, type, options);
if (req->pathlen >= MAXPATHLEN)
return (-1);
Index: server/gam_inotify.c
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_inotify.c,v
retrieving revision 1.28
diff -u -r1.28 gam_inotify.c
--- server/gam_inotify.c 21 Jul 2005 17:34:34 -0000 1.28
+++ server/gam_inotify.c 25 Jul 2005 22:34:34 -0000
@@ -234,6 +234,7 @@
/* We aren't already watching this path */
path_wd = gam_inotify_add_watch (path, should_poll_mask);
if (path_wd < 0) {
+ GAM_DEBUG (DEBUG_INFO, "inotify: could not add watch for %s\n", path);
G_UNLOCK(inotify);
return;
}
@@ -360,17 +361,12 @@
static void
gam_inotify_directory_handler(const char *path, pollHandlerMode mode)
{
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_directory_handler %s : %d\n",
- path, mode);
-
gam_inotify_directory_handler_internal(path, mode);
}
static void
gam_inotify_file_handler(const char *path, pollHandlerMode mode)
{
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_file_handler %s : %d\n", path, mode);
-
if (g_file_test(path, G_FILE_TEST_IS_DIR)) {
gam_inotify_directory_handler_internal(path, mode);
} else {
@@ -450,7 +446,6 @@
static gboolean
gam_inotify_consume_subscriptions_real(gpointer data)
{
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_consume_subscriptions_real()\n");
gam_poll_consume_subscriptions();
have_consume_idler = FALSE;
return FALSE;
@@ -464,7 +459,6 @@
if (have_consume_idler)
return;
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_consume_subscriptions()\n");
have_consume_idler = TRUE;
source = g_idle_source_new();
g_source_set_callback(source, gam_inotify_consume_subscriptions_real,
@@ -545,15 +539,12 @@
gboolean
gam_inotify_add_subscription(GamSubscription * sub)
{
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_add_subscription\n");
-
if (!gam_poll_add_subscription(sub)) {
return FALSE;
}
gam_inotify_consume_subscriptions();
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_add_subscription: done\n");
return TRUE;
}
@@ -566,7 +557,6 @@
gboolean
gam_inotify_remove_subscription(GamSubscription * sub)
{
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_remove_subscription\n");
if (!gam_poll_remove_subscription(sub)) {
return FALSE;
@@ -574,7 +564,6 @@
gam_inotify_consume_subscriptions();
- GAM_DEBUG(DEBUG_INFO, "gam_inotify_remove_subscription: done\n");
return TRUE;
}
Index: server/gam_listener.c
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_listener.c,v
retrieving revision 1.8
diff -u -r1.8 gam_listener.c
--- server/gam_listener.c 13 Jun 2005 16:36:28 -0000 1.8
+++ server/gam_listener.c 25 Jul 2005 22:34:34 -0000
@@ -24,11 +24,16 @@
#include "gam_subscription.h"
#include "gam_server.h"
#include "gam_error.h"
+#include "gam_pidname.h"
+
+// #define GAM_LISTENER_VERBOSE
+
/* private struct representing a single listener */
struct _GamListener {
void *service;
int pid;
+ char *pidname;
GList *subs;
};
@@ -61,9 +66,12 @@
listener = g_new0(GamListener, 1);
listener->service = service;
listener->pid = pid;
+ listener->pidname = gam_get_pidname (pid);
listener->subs = NULL;
- GAM_DEBUG(DEBUG_INFO, "Created listener for %d\n", pid);
+#ifndef GAM_LISTENER_VERBOSE
+ GAM_DEBUG(DEBUG_INFO, "Created listener for %s\n", listener->pidname);
+#endif
return listener;
}
@@ -100,13 +108,14 @@
g_assert(listener);
- GAM_DEBUG(DEBUG_INFO, "Freeing listener for %d\n", listener->pid);
+ GAM_DEBUG(DEBUG_INFO, "Freeing listener for %s\n", listener->pidname);
while ((cur = g_list_first(listener->subs)) != NULL) {
GamSubscription * sub = cur->data;
gam_listener_free_subscription(listener, sub);
listener->subs = g_list_delete_link(listener->subs, cur);
}
+ g_free (listener->pidname);
g_free(listener);
}
@@ -142,6 +151,21 @@
}
/**
+ * gam_listener_get_pidname:
+ *
+ * @listener: the #GamListener
+ *
+ * Gets the process name associated with a #GamListener
+ *
+ * Returns the process name associated with the #GamListener.
+ */
+const char *
+gam_listener_get_pidname(GamListener *listener)
+{
+ return listener->pidname;
+}
+
+/**
* gam_listener_get_subscription:
*
* @listener: the #GamListener
@@ -225,6 +249,7 @@
g_assert(!g_list_find(listener->subs, sub));
listener->subs = g_list_prepend(listener->subs, sub);
+ GAM_DEBUG(DEBUG_INFO, "Adding sub %s to listener %s\n", gam_subscription_get_path (sub), listener->pidname);
}
/**
@@ -244,7 +269,7 @@
g_assert(g_list_find(listener->subs, sub));
listener->subs = g_list_remove(listener->subs, sub);
-
+ GAM_DEBUG(DEBUG_INFO, "Removing sub %s from listener %s\n", gam_subscription_get_path (sub), listener->pidname);
/* There should only be one. */
g_assert(!g_list_find(listener->subs, sub));
}
Index: server/gam_listener.h
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_listener.h,v
retrieving revision 1.4
diff -u -r1.4 gam_listener.h
--- server/gam_listener.h 13 Jun 2005 16:36:28 -0000 1.4
+++ server/gam_listener.h 25 Jul 2005 22:34:34 -0000
@@ -18,6 +18,7 @@
void *gam_listener_get_service (GamListener *listener);
int gam_listener_get_pid (GamListener *listener);
+const char * gam_listener_get_pidname (GamListener *listener);
void gam_listener_add_subscription (GamListener *listener,
GamSubscription *sub);
Index: server/gam_poll.c
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_poll.c,v
retrieving revision 1.61
diff -u -r1.61 gam_poll.c
--- server/gam_poll.c 21 Jul 2005 18:15:37 -0000 1.61
+++ server/gam_poll.c 25 Jul 2005 22:34:34 -0000
@@ -60,6 +60,7 @@
static GamPollHandler dir_handler = NULL;
static GamPollHandler file_handler = NULL;
static pollHandlerKernel type_khandler = GAMIN_K_NONE;
+G_LOCK_DEFINE_STATIC(poll_lock);
static int poll_mode = 0;
@@ -457,6 +458,7 @@
GAM_DEBUG(DEBUG_INFO, "Poll: poll_file for %s called\n", path);
#endif
+ memset(&sbuf, 0, sizeof(struct stat));
if (node->lasttime == 0) {
GAM_DEBUG(DEBUG_INFO, "Poll: file is new\n");
stat_ret = stat(node->path, &sbuf);
@@ -466,7 +468,7 @@
gam_node_set_is_dir(node, (S_ISDIR(sbuf.st_mode) != 0));
if (gam_exclude_check(path))
node->pflags |= MON_NOKERNEL;
- memcpy(&(node->sbuf), &(sbuf), sizeof(sbuf));
+ memcpy(&(node->sbuf), &(sbuf), sizeof(struct stat));
node->lasttime = current_time;
if (stat_ret == 0)
@@ -530,7 +532,9 @@
*/
if (stat_ret == 0)
gam_node_set_is_dir(node, (S_ISDIR(sbuf.st_mode) != 0));
- node->sbuf = sbuf;
+
+ memcpy(&(node->sbuf), &(sbuf), sizeof(struct stat));
+ node->sbuf.st_mtime = sbuf.st_mtime; // VALGRIND!
/*
* if kernel monitoring prohibited, stop here
@@ -544,7 +548,7 @@
*/
if (current_time == node->lasttime) {
if (!(node->pflags & MON_BUSY)) {
- if (node->sbuf.st_mtime == current_time)
+ if (sbuf.st_mtime == current_time)
node->checks++;
}
} else {
@@ -1058,8 +1062,9 @@
* make sure the subscription still isn't in the new subscription queue
*/
if (g_list_find(new_subs, sub)) {
- GAM_DEBUG(DEBUG_INFO, "new subscriptions is removed\n");
+ GAM_DEBUG(DEBUG_INFO, "new subscription is removed\n");
new_subs = g_list_remove_all(new_subs, sub);
+ return TRUE;
}
node = gam_tree_get_at_path(tree, gam_subscription_get_path(sub));
Index: server/gam_subscription.c
===================================================================
RCS file: /cvs/gnome/gamin/server/gam_subscription.c,v
retrieving revision 1.14
diff -u -r1.14 gam_subscription.c
--- server/gam_subscription.c 9 May 2005 13:45:55 -0000 1.14
+++ server/gam_subscription.c 25 Jul 2005 22:34:34 -0000
@@ -28,6 +28,8 @@
#include "gam_event.h"
#include "gam_error.h"
+// #define GAM_SUB_VERBOSE
+
struct _GamSubscription {
char *path;
int events;
@@ -81,7 +83,9 @@
sub->is_dir = is_dir;
sub->options = options;
+#ifdef GAM_SUB_VERBOSE
GAM_DEBUG(DEBUG_INFO, "Created subscription for %s\n", path);
+#endif
return sub;
}
@@ -95,7 +99,9 @@
{
if (sub == NULL)
return;
+#ifdef GAM_SUB_VERBOSE
GAM_DEBUG(DEBUG_INFO, "Freeing subscription for %s\n", sub->path);
+#endif
g_free(sub->path);
g_free(sub);
@@ -183,7 +189,7 @@
{
if (sub == NULL)
return;
- GAM_DEBUG(DEBUG_INFO, "Setting subscription listener for %s\n", sub->path);
+ GAM_DEBUG(DEBUG_INFO, "%s listening for %s\n", gam_listener_get_pidname (listener), sub->path);
sub->listener = listener;
}
@@ -253,7 +259,7 @@
{
if (sub == NULL)
return;
- GAM_DEBUG(DEBUG_INFO, "Cancelling subscription for %s\n", sub->path);
+ GAM_DEBUG(DEBUG_INFO, "%s not listening for %s\n", gam_listener_get_pidname (sub->listener), sub->path);
sub->cancelled = TRUE;
}
#include <config.h>
#include <stdio.h>
#include <glib.h>
#include "gam_error.h"
#include "gam_pidname.h"
char *gam_get_pidname (int pid)
{
#ifdef HAVE_LINUX
gchar *procname;
gchar *pidname = NULL;
FILE *fp;
#endif
#ifdef HAVE_LINUX
procname = g_strdup_printf ("/proc/%d/cmdline", pid);
fp = fopen(procname, "r");
g_free (procname);
if (!fp) {
pidname = g_strdup_printf ("%d", pid);
} else {
gchar *name = g_malloc (128);
int i = 0;
while (i < 128) {
int ch = fgetc (fp);
if (ch == EOF)
break;
name[i++] = ch;
if (ch == '\0')
break;
}
name[127] = '\0';
pidname = g_strdup (name);
g_free (name);
fclose (fp);
}
#else
pidname = g_strdup_printf ("%d", pid);
#endif
return pidname;
}
/* Gam PID name */
#ifndef __GAM_PIDNAME
#define __GAM_PIDNAME
char *gam_get_pidname (int pid);
#endif
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
