[Evolution-hackers] [PATCH] Remove unused KILL_PROCESS_CMD detection
Hey,
I noticed that the killall/pkill-based solution for killev has been
rewritten and doesn't use killall anymore, therefore I took the liberty
to get rid of it's detection in configure.ac and also removed
corresponding IFDEF in code.
It would be nice if someone could check whether WIN32 still compiles..
From 18ff2b2adb08150cde2309efec6c5aaa96cb6387 Mon Sep 17 00:00:00 2001
From: Priit Laes pl...@plaes.org
Date: Fri, 26 Feb 2010 12:46:27 +0200
Subject: [PATCH] Remove unused KILL_PROCESS_CMD stuff.
---
configure.ac | 19 ---
shell/main.c |2 --
2 files changed, 0 insertions(+), 21 deletions(-)
diff --git a/configure.ac b/configure.ac
index 2b93435..1b51462 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1042,25 +1042,6 @@ AC_SUBST(MANUAL_NSPR_LIBS)
AC_SUBST(MANUAL_NSS_CFLAGS)
AC_SUBST(MANUAL_NSS_LIBS)
-dnl *
-dnl killall or pkill?
-dnl *
-
-case $host in
-*solaris*)
- KILL_PROCESS_CMD=/usr/bin/pkill -x
- ;;
-*)
- AC_PATH_PROGS(KILL_PROCESS_CMD, killall pkill)
- ;;
-esac
-if test -z $KILL_PROCESS_CMD; then
- AC_MSG_WARN([Could not find a command to kill a process by name])
-else
- AC_DEFINE_UNQUOTED([KILL_PROCESS_CMD], $KILL_PROCESS_CMD,
- [Command to kill processes by name])
-fi
-
dnl ***
dnl GObject marshalling
dnl ***
diff --git a/shell/main.c b/shell/main.c
index 72e9aed..2c326f2 100644
--- a/shell/main.c
+++ b/shell/main.c
@@ -322,10 +322,8 @@ static GOptionEntry entries[] = {
N_(Start in offline mode), NULL },
{ online, '\0', 0, G_OPTION_ARG_NONE, start_online,
N_(Start in online mode), NULL },
-#ifdef KILL_PROCESS_CMD
{ force-shutdown, '\0', 0, G_OPTION_ARG_NONE, force_shutdown,
N_(Forcibly shut down Evolution), NULL },
-#endif
#ifdef DEVELOPMENT
{ force-migrate, '\0', 0, G_OPTION_ARG_NONE, force_migrate,
N_(Forcibly re-migrate from Evolution 1.4), NULL },
--
1.7.0
___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers
[Evolution-hackers] camel-folder-summary locking ...
Hi guys,
I just hit a nasty in camel-folder-summary; suggested patch attached,
seemingly a simple problem of a re-enterancy hazard in the same thread
with a ghashtable:
Thread 121 (Thread 0xa7f37b70 (LWP 23631)):
#0 0xb68c13ef in g_hash_table_resize (hash_table=value optimized out) at
ghash.c:424
#1 g_hash_table_maybe_resize (hash_table=value optimized out) at ghash.c:457
#2 g_hash_table_remove_internal (hash_table=value optimized out) at
ghash.c:982
#3 0xb7c7fdba in message_info_free (s=0x8e7e438, info=0x90f89f0) at
camel-folder-summary.c:3497
#4 0xb7c7fe75 in camel_message_info_free (o=0x90f89f0) at
camel-folder-summary.c:4518
#5 0xb7c7ffce in remove_item (key=0x9142618 217647, info=0x90f89f0,
s=0x8e7e438) at camel-folder-summary.c:788
#6 0xb68c0b2c in g_hash_table_foreach_remove_or_steal (hash_table=0x8f7d0c0 =
{...}, func=0xb7c7ff40 remove_item, user_data=0x8e7e438, notify=1)
at ghash.c:1109
#7 0xb7c85356 in remove_cache (session=0x80ce680, msg=0xb093250) at
camel-folder-summary.c:818
#8 0xb7ca0162 in session_thread_proxy (msg=0xb093250, session=0x80ce680) at
camel-session.c:590
#9 0xb68fb036 in g_thread_pool_thread_proxy (data=0x8aa0cb8) at
gthreadpool.c:265
---Type return to continue, or q return to quit---
#10 0xb68f99ef in g_thread_create_proxy (data=0xb0c1ee8) at gthread.c:635
#11 0xb764c725 in start_thread () from /lib/libpthread.so.0
#12 0xb680626e in clone () from /lib/libc.so.6
Having said that, the code looks a bit nervous (judging by the
comments) in this area. Review much appreciated etc. Reading through the
code there though, I too began to get a bit nervous trying to keep track
of what locks are held.
eg.
camel_folder_summary_migrate_infos
+ doesn't seem to take the summary_lock - why ?
likewise 'message_info_free'
+ called from camel_message_info_free
+ doesn't seem to take the lock ... - why ?
if it is just assumed then:
+ most callers of camel_message_info_free *have* that lock,
+ except eg. camel_folder_summary_remove ...
+ or eg. camel_folder_summary_remove_index_fast
+ or summary_header_save, or message_info_load,
+ or msg_update_preview,
+ or camel_folder_summary_migrate_infos
Soo ... if you read the GType / GObject code - which is similarly
twisted / complicated - then it uses name mangling to denote what lock
is held (personally, I rather like that). Thus you can see by reading
it:
do_foo()
{
take_lock();
do_foo_T();
release_lock();
}
is safe, and
do_foo_T() { do_baa(); }
is essentially not :-) (the '_T' suffix needs propagating). ORBit2 does
the same where things get sticky.
Failing that, the linux kernel seems in places to use comments of
the form:
* The files-file_lock should be held on entry, and will be held on exit.
Clearly, something to reduce the barrier to entry here such that the
code is more readable would be good ;-)
HTH,
Michael.
--
michael.me...@novell.com , Pseudo Engineer, itinerant idiot
diff --git a/camel/camel-folder-summary.c b/camel/camel-folder-summary.c
index 85265ac..787f229 100644
--- a/camel/camel-folder-summary.c
+++ b/camel/camel-folder-summary.c
@@ -778,18 +778,15 @@ cfs_count_dirty (CamelFolderSummary *s)
/* FIXME[disk-summary] I should have a better LRU algorithm */
static gboolean
-remove_item (gchar *key, CamelMessageInfoBase *info, CamelFolderSummary *s)
+remove_item (gchar *key, CamelMessageInfoBase *info, GSList **to_free_list)
{
d(printf(%d(%d)\t, info-refcount, info-dirty)); /* camel_message_info_dump (info); */
CAMEL_SUMMARY_LOCK(info-summary, ref_lock);
if (info-refcount == 1 !info-dirty !(info-flags CAMEL_MESSAGE_FOLDER_FLAGGED)) {
CAMEL_SUMMARY_UNLOCK(info-summary, ref_lock);
- /* Hackit so that hashtable isn;t corrupted. */
- /* FIXME: These uid strings are not yet freed. We should get this done soon. */
- camel_pstring_free (info-uid);
- info-uid = NULL;
- /* Noone seems to need it. Why not free it then. */
- camel_message_info_free (info);
+
+ /* free the entry later */
+ *to_free_list = g_slist_prepend (*to_free_list, info);
return TRUE;
}
CAMEL_SUMMARY_UNLOCK(info-summary, ref_lock);
@@ -807,6 +804,7 @@ remove_cache (CamelSession *session, CamelSessionThreadMsg *msg)
struct _folder_summary_free_msg *m = (struct _folder_summary_free_msg *)msg;
CamelFolderSummary *s = m-summary;
CamelException ex;
+ GSList *to_free, *l;
CAMEL_DB_RELEASE_SQLITE_MEMORY;
camel_exception_init (ex);
@@ -819,7 +817,14 @@ remove_cache (CamelSession *session, CamelSessionThreadMsg *msg)
dd(printf(removing cache for %s %d %p\n, s-folder ? s-folder-full_name : s-summary_path, g_hash_table_size (s-loaded_infos), (gpointer)
[Evolution-hackers] edbus branch review ...
Hi guys, I have a new fun branch (to review for merging) mmeeks-gdbus-import So I am wickedly piling up other misc. fixes in there (so they don't mask other issues) - but I have the edbus code imported, and running - and, indeed, it seems to block rather less nastily than the dbus-glib variants[1]: which is all good. It is essentially the work from origin/treitter-client-gdbus but adding the 'refresh' method, and of course porting it all to the internal copy of 'libedbus.so'. This should let us stick with glib-2.28 (what I'm using), and yet get the benefits of moving closer to what gdbus will be in the end (I hope) - as well as the interactivity wins. I incorporate the fairly simple perl script I used to do the substitution in the first instance. Why am I telling you this ? - I'd love some review of course; I've been playing with calendar addressbook here for a bit; but would greatly appreciate some more input. Particularly since I didn't write the original gdbus port. I'd also -really- like to get this in soonish; Thoughts / flameage ? :-) Thanks, Michael. [1] - which seems to like blocking the mainloop any time any other thread does a synchronous call (not optimal). -- michael.me...@novell.com , Pseudo Engineer, itinerant idiot This is a version of David Zeuthan's gdbus code, intended for inclusion in glib. Until it is included, it helps us to have a copy to work with included in Evolution. Of course, some regexps are required to rename g_dbus - e_dbus etc. to avoid potential symbol conflicts. The code is taken from: git://anongit.freedesktop.org/~david/gdbus-standalone commit 7457cdc8863f08163c48de455e8cd7469b0fe876 With a number of fixes layered on top. Also included is a renamed copy of gvariant from glib commit 1433655e23634446b990d1cf782b22ad6430496a perl script to in-place re-write code using g_dbus: #!/usr/bin/perl -pi.bak s/G_VARIANT/E_VARIANT/g; s/G_TYPE_VARIANT/E_TYPE_VARIANT/g; s/GVariant/EVariant/g; s/g_variant/e_variant/g; s/g_bit_/e_bit_/g; s/g_bus_/e_bus_/g; s/GDBus/EDBus/g; s/G_DBUS_/E_DBUS_/g; s/G_TYPE_DBUS/E_TYPE_DBUS/g; s/g_dbus_/e_dbus_/g; ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
Re: [Evolution-hackers] [PATCH] Remove unused KILL_PROCESS_CMD detection
On Fri, 2010-02-26 at 17:22 +0200, Priit Laes wrote: I noticed that the killall/pkill-based solution for killev has been rewritten and doesn't use killall anymore, therefore I took the liberty to get rid of it's detection in configure.ac and also removed corresponding IFDEF in code. It would be nice if someone could check whether WIN32 still compiles.. I can't speak for Win32 or even Unices beyond Linux, but the patch looks okay to me. Being so close to a stable release, however, unless this fixes a bug I'd suggest holding off until 2.31 development opens up. Matthew Barnes ___ Evolution-hackers mailing list Evolution-hackers@gnome.org http://mail.gnome.org/mailman/listinfo/evolution-hackers
