Hi Daniel,
I spent some time last week rewriting gam_poll.c. The major problem
that I have with the current implementation is its sloppy use of
state: the MON_ flags are set and cleared without regard to their
current state; the code often does not distinguish between directory
events sent to file subscriptions and vice versa (and consider
gam_node_has_dir_subscriptions which assumes that if the node is not a
directory it doesn't have any directory subscriptions!); there are a
number of gratuitous checks e.g. with gam_exclude_check which only
needs to be run once when the node is created; O(n) algorithms when
there are O(1) algorithms available e.g. sending one DESACTIVATE event
per subscription rather than per node.
I've also removed the gam_poll_consume_subscriptions interface: the
poll back end does it on its own and neither the dnotify nor the
inotify back end need the idle source. It is possible to watch the
root of the file systems thus looking up / with gam_tree_get_at_path
needs to work.
The attached patch will apply against CVS (with two bits of fuzz but
that can be ignored).
test suite
----------
There are a couple of make check failures when using the dnotify
backend:
basic4.py: monitors a directory and expects change events on changes
of files in the directory. The SGI documentation is clear
that change events are only sent if the stat information
changes. This is not the case if a file changes.
dnotify.py: does a watch_file but should do a watch_directory
dnotify6.py: monitors a missing file then creates a directory with the
same path and expects a create event. I think this is
incorrect: a create event should only be sent to file
subscriptions
dnotify12.py: similar problem as basic4.py
dnotify13.py: get an extra change event on temp_dir/a. I've traced it
and it seems legitimate: there is one event for a file size
change and one for an mtime change. I am not entirely
clear what is happening, however it may have something to
do with the way python writes to the file.
flood4.py: there is a timing problem: the thread creates the
directories before gamin does the initial scan. Creating
the thread after we get the Exist events on the directory
fixes this.
4.tst: same problem as dnotify6.py
9.tst: same problem as dnotify.py
The poll back end has the same failures plus a couple more. First,
the poll back end tends to generate a few more change events on
directories than the dnotify back end. I think this is more a
deficiency of the dnotify backend rather than the poll backend. Also,
timing may have something to do with it: the poll backend only checks
for changes once per second which may cause events to coalesce.
stability
---------
As far as I can tell there are no node or subscription leaks (when
there are no extant subscriptions, the only node is the root node).
valgrind reports no memory loss. It seems to perform well under
pressure: I have left it running overnight without any crashes (using
a while true; do make check; done loop). I submitted it to additional
pressure by configuring three build trees and running the make check
loop in the python directory and a fourth make check loop being
careful to exclude test 4 in one of the C test directories (which uses
global state). It acts well for a few hours, however, it evenutally
gets a SIGSEGV. I ran it under valgrind and got the same result.
From what I can gather, that the problem is that GQueue is not signal
safe.
valgrind output:
gam_dnotify_remove_subscription: done
Closing connection 9
gam_dnotify_pipe_handler()
==2197== Invalid read of size 4
==2197== at 0x1B94D827: g_queue_pop_tail (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80548E7: gam_dnotify_pipe_handler (gam_dnotify.c:303)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93FED2: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x804B7D7: main (gam_server.c:350)
==2197== Address 0x1BE51D38 is 8 bytes inside a block of size 12 free'd
==2197== at 0x1B904B04: free (vg_replace_malloc.c:152)
==2197== by 0x1B944A83: g_free (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80521A1: g_list_free_1 (gam_debug_lists.c:59)
==2197== by 0x80522E5: g_list_delete_link (gam_debug_lists.c:162)
==2197== by 0x804ABF8: gam_listener_free (gam_listener.c:108)
==2197== by 0x8050758: gam_connection_close (gam_connection.c:85)
==2197== by 0x8050146: gam_client_conn_shutdown (gam_channel.c:690)
==2197== by 0x804FCA7: gam_client_conn_read (gam_channel.c:274)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197==
==2197== Invalid read of size 4
==2197== at 0x1B94D82A: g_queue_pop_tail (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80548E7: gam_dnotify_pipe_handler (gam_dnotify.c:303)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93FED2: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x804B7D7: main (gam_server.c:350)
==2197== Address 0x1BE51D30 is 0 bytes inside a block of size 12 free'd
==2197== at 0x1B904B04: free (vg_replace_malloc.c:152)
==2197== by 0x1B944A83: g_free (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80521A1: g_list_free_1 (gam_debug_lists.c:59)
==2197== by 0x80522E5: g_list_delete_link (gam_debug_lists.c:162)
==2197== by 0x804ABF8: gam_listener_free (gam_listener.c:108)
==2197== by 0x8050758: gam_connection_close (gam_connection.c:85)
==2197== by 0x8050146: gam_client_conn_shutdown (gam_channel.c:690)
==2197== by 0x804FCA7: gam_client_conn_read (gam_channel.c:274)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in
/usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197==
==2197== Invalid write of size 4
==2197== at 0x1B94D833: g_queue_pop_tail (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80548E7: gam_dnotify_pipe_handler (gam_dnotify.c:303)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93FED2: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x804B7D7: main (gam_server.c:350)
==2197== Address 0x3 is not stack'd, malloc'd or (recently) free'd
==2197==
==2197== Process terminating with default action of signal 11
(SIGSEGV)
==2197== Access not within mapped region at address 0x3
==2197== at 0x1B94D833: g_queue_pop_tail (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x80548E7: gam_dnotify_pipe_handler (gam_dnotify.c:303)
==2197== by 0x1B963DBE: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93E581: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F5F7: g_main_context_dispatch (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93F92F: (within /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x1B93FED2: g_main_loop_run (in /usr/lib/libglib-2.0.so.0.600.4)
==2197== by 0x804B7D7: main (gam_server.c:350)
Thanks,
Neal
Attachment:
gam-poll.diff
Description: Binary data