[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[Libevent-users] event_debug_mode_too_late assignment in event.c subject to race conditions



Hi,

While running valgrind's drd on my app, I ran into a bunch of race
conditions on the global variable event_debug_mode_too_late.  The
questionable assignment happens in a bunch of macros starting on
event.c:195 in libevent-2.0.8-rc

The two threads involved are a server thread and a client thread which
share no event_base.  Their event loops are separate; they communicate
only through the proper channels, but I was getting races between
event_del() in one thread and various other locations in the other
thread.

I have an ugly patch below which moves the event_debug_mode_too_late
assignment in event.c inside the EVLOCK_UNLOCK( _event_debug_map_lock,
0 ), which makes all of these race conditions disappear from
valgrind's drd, but I am not sure if it is correct to patch in this
manner.

- Ken

*** event.c     2010-11-19 03:42:03.838376765 +0800
--- event.new.c 2010-11-19 03:41:48.377127426 +0800
***************
*** 209,217 ****
                        dent->added = 0;                                \
                        HT_INSERT(event_debug_map, &global_debug_map, dent); \
                }                                                       \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
-       event_debug_mode_too_late = 1;                                  \
        } while (0)
  /* Macro: record that ev is no longer setup */
  #define _event_debug_note_teardown(ev) do {                           \
--- 209,217 ----
                        dent->added = 0;                                \
                        HT_INSERT(event_debug_map, &global_debug_map, dent); \
                }                                                       \
+       event_debug_mode_too_late = 1;                                  \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
        } while (0)
  /* Macro: record that ev is no longer setup */
  #define _event_debug_note_teardown(ev) do {                           \
***************
*** 222,230 ****
                dent = HT_REMOVE(event_debug_map, &global_debug_map, &find); \
                if (dent)                                               \
                        mm_free(dent);                                  \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
-       event_debug_mode_too_late = 1;                                  \
        } while (0)
  /* Macro: record that ev is now added */
  #define _event_debug_note_add(ev)     do {                            \
--- 222,230 ----
                dent = HT_REMOVE(event_debug_map, &global_debug_map, &find); \
                if (dent)                                               \
                        mm_free(dent);                                  \
+       event_debug_mode_too_late = 1;                          \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
        } while (0)
  /* Macro: record that ev is now added */
  #define _event_debug_note_add(ev)     do {                            \
***************
*** 240,248 ****
                            "%s: noting an add on a non-setup event %p", \
                            __func__, (ev));                            \
                }                                                       \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
-       event_debug_mode_too_late = 1;                                  \
        } while (0)
  /* Macro: record that ev is no longer added */
  #define _event_debug_note_del(ev) do {
         \
--- 240,248 ----
                            "%s: noting an add on a non-setup event %p", \
                            __func__, (ev));                            \
                }                                                       \
+       event_debug_mode_too_late = 1;                                  \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
        } while (0)
  /* Macro: record that ev is no longer added */
  #define _event_debug_note_del(ev) do {
         \
***************
*** 258,266 ****
                            "%s: noting a del on a non-setup event %p", \
                            __func__, (ev));                            \
                }                                                       \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
-       event_debug_mode_too_late = 1;                                  \
        } while (0)
  /* Macro: assert that ev is setup (i.e., okay to add or inspect) */
  #define _event_debug_assert_is_setup(ev) do {                         \
--- 258,266 ----
                            "%s: noting a del on a non-setup event %p", \
                            __func__, (ev));                            \
                }                                                       \
+       event_debug_mode_too_late = 1;                                  \
                EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
        }                                                               \
        } while (0)
  /* Macro: assert that ev is setup (i.e., okay to add or inspect) */
  #define _event_debug_assert_is_setup(ev) do {                         \
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.