[pypy-commit] pypy reverse-debugger: Starting thread support: RPY_REVDB_EMIT() should be called mostly when

arigo pypy.commits at gmail.com
Tue Aug 9 10:46:18 EDT 2016


Author: Armin Rigo <arigo at tunes.org>
Branch: reverse-debugger
Changeset: r86107:8009adcd405a
Date: 2016-08-09 16:45 +0200
http://bitbucket.org/pypy/pypy/changeset/8009adcd405a/

Log:	Starting thread support: RPY_REVDB_EMIT() should be called mostly
	when we hold the GIL, but ensuring that is messy. For now, we simply
	use our own lock here.

diff --git a/rpython/translator/revdb/src-revdb/revdb.c b/rpython/translator/revdb/src-revdb/revdb.c
--- a/rpython/translator/revdb/src-revdb/revdb.c
+++ b/rpython/translator/revdb/src-revdb/revdb.c
@@ -11,6 +11,7 @@
 #include <ctype.h>
 #include <signal.h>
 #include <search.h>
+#include <sched.h>
 
 #ifdef __linux__
 #  define HAVE_PERSONALITY
@@ -119,17 +120,27 @@
         setup_record_mode(*argc_p, *argv_p);
 }
 
+static void reverse_db_lock_and_flush(void)
+{
+    _RPY_REVDB_LOCK();
+    rpy_reverse_db_flush();
+    _RPY_REVDB_UNLOCK();
+}
+
 RPY_EXTERN
 void rpy_reverse_db_teardown(void)
 {
     uint64_t stop_points;
-    if (RPY_RDB_REPLAY) {
+    if (!RPY_RDB_REPLAY) {
+        _RPY_REVDB_LOCK();
+    }
+    else {
         /* hack: prevents RPY_REVDB_EMIT() from calling
            rpy_reverse_db_fetch(), which has nothing more to fetch now */
         rpy_revdb.buf_limit += 1;
     }
-    RPY_REVDB_EMIT(stop_points = rpy_revdb.stop_point_seen; ,
-                   uint64_t _e, stop_points);
+    _RPY_REVDB_EMIT_L(stop_points = rpy_revdb.stop_point_seen; ,
+                      uint64_t _e, stop_points, /*must_lock=*/0);
 
     if (!RPY_RDB_REPLAY) {
         rpy_reverse_db_flush();
@@ -137,6 +148,7 @@
             close(rpy_rev_fileno);
             rpy_rev_fileno = -1;
         }
+        _RPY_REVDB_UNLOCK();
     }
     else
         check_at_end(stop_points);
@@ -207,7 +219,7 @@
                     filename);
             abort();
         }
-        atexit(rpy_reverse_db_flush);
+        atexit(reverse_db_lock_and_flush);
 
         write_all(RDB_SIGNATURE, strlen(RDB_SIGNATURE));
         for (i = 0; i < argc; i++) {
@@ -244,8 +256,12 @@
 
 static void flush_buffer(void)
 {
+    /* must be called with the lock held */
+    ssize_t full_size;
+    assert(rpy_revdb.lock);
+
     /* write the current buffer content to the OS */
-    ssize_t full_size = rpy_revdb.buf_p - rpy_rev_buffer;
+    full_size = rpy_revdb.buf_p - rpy_rev_buffer;
     rpy_revdb.buf_p = rpy_rev_buffer + sizeof(int16_t);
     if (rpy_rev_fileno >= 0)
         write_all(rpy_rev_buffer, full_size);
@@ -253,13 +269,18 @@
 
 static ssize_t current_packet_size(void)
 {
+    /* must be called with the lock held */
     return rpy_revdb.buf_p - (rpy_rev_buffer + sizeof(int16_t));
 }
 
 RPY_EXTERN
 void rpy_reverse_db_flush(void)
 {
-    ssize_t content_size = current_packet_size();
+    /* must be called with the lock held */
+    ssize_t content_size;
+    assert(rpy_revdb.lock);
+
+    content_size = current_packet_size();
     if (content_size != 0) {
         char *p = rpy_rev_buffer;
         assert(0 < content_size && content_size <= 32767);
@@ -268,6 +289,18 @@
     }
 }
 
+RPY_EXTERN
+void rpy_reverse_db_lock_acquire(void)
+{
+    while (1) {
+        if (rpy_revdb.lock == 0) {
+            if (pypy_lock_test_and_set(&rpy_revdb.lock, 1) == 0)
+                break;   /* done */
+        }
+        sched_yield();
+    }
+}
+
 void boehm_gc_finalizer_notifier(void)
 {
     /* This is called by Boehm when there are pending finalizers.
@@ -303,6 +336,7 @@
     int64_t done;
 
     /* Write an ASYNC_FINALIZER_TRIGGER packet */
+    _RPY_REVDB_LOCK();
     rpy_reverse_db_flush();
     assert(current_packet_size() == 0);
 
@@ -310,6 +344,7 @@
     memcpy(rpy_revdb.buf_p, &rpy_revdb.stop_point_seen, sizeof(uint64_t));
     rpy_revdb.buf_p += sizeof(uint64_t);
     flush_buffer();
+    _RPY_REVDB_UNLOCK();
 
     /* Invoke all Boehm finalizers.  For new-style finalizers, this
        will only cause them to move to the queues, where
@@ -364,8 +399,10 @@
 
 static uint64_t recording_offset(void)
 {
+    /* must be called with the lock held */
     off_t base_offset;
     ssize_t extra_size = rpy_revdb.buf_p - rpy_rev_buffer;
+    assert(rpy_revdb.lock);
 
     if (rpy_rev_fileno < 0)
         return 1;
@@ -379,7 +416,10 @@
 
 static void patch_prev_offset(int64_t offset, char old, char new)
 {
+    /* must be called with the lock held */
     off_t base_offset;
+    assert(rpy_revdb.lock);
+
     if (rpy_rev_fileno < 0)
         return;
     base_offset = lseek(rpy_rev_fileno, 0, SEEK_CUR);
@@ -452,14 +492,18 @@
         /* Emit WEAKREF_AFTERWARDS_DEAD, but remember where we emit it.
            If we deref the weakref and it is still alive, we will patch
            it with WEAKREF_AFTERWARDS_ALIVE. */
-        if (!RPY_RDB_REPLAY)
+        if (!RPY_RDB_REPLAY) {
+            _RPY_REVDB_LOCK();
             r->re_off_prev = recording_offset();
+        }
         else
             r->re_off_prev = 1;    /* any number > 0 */
 
-        RPY_REVDB_EMIT(alive = WEAKREF_AFTERWARDS_DEAD;, char _e, alive);
+        _RPY_REVDB_EMIT_L(alive = WEAKREF_AFTERWARDS_DEAD;, char _e, alive,
+                          /*must_lock=*/0);
 
         if (!RPY_RDB_REPLAY) {
+            _RPY_REVDB_UNLOCK();
             OP_BOEHM_DISAPPEARING_LINK(&r->re_addr, target, /*nothing*/);
         }
         else {
@@ -498,13 +542,18 @@
         else {
             char alive;
             if (!RPY_RDB_REPLAY) {
+                _RPY_REVDB_LOCK();
                 patch_prev_offset(r->re_off_prev, WEAKREF_AFTERWARDS_DEAD,
                                                   WEAKREF_AFTERWARDS_ALIVE);
                 r->re_off_prev = recording_offset();
             }
-            RPY_REVDB_EMIT(alive = WEAKREF_AFTERWARDS_DEAD;, char _e, alive);
+            _RPY_REVDB_EMIT_L(alive = WEAKREF_AFTERWARDS_DEAD;, char _e, alive,
+                              /*must_lock=*/0);
 
-            if (RPY_RDB_REPLAY) {
+            if (!RPY_RDB_REPLAY) {
+                _RPY_REVDB_UNLOCK();
+            }
+            else {
                 switch (alive) {
                 case WEAKREF_AFTERWARDS_DEAD:
                     r->re_addr = NULL;
@@ -527,8 +576,10 @@
     locnum += 300;
     assert(locnum < 0xFC00);
     if (!RPY_RDB_REPLAY) {
-        _RPY_REVDB_EMIT_RECORD(unsigned char _e, (locnum >> 8));
-        _RPY_REVDB_EMIT_RECORD(unsigned char _e, (locnum & 0xFF));
+        _RPY_REVDB_LOCK();
+        _RPY_REVDB_EMIT_RECORD_L(unsigned char _e, (locnum >> 8));
+        _RPY_REVDB_EMIT_RECORD_L(unsigned char _e, (locnum & 0xFF));
+        _RPY_REVDB_UNLOCK();
     }
 }
 
diff --git a/rpython/translator/revdb/src-revdb/revdb_include.h b/rpython/translator/revdb/src-revdb/revdb_include.h
--- a/rpython/translator/revdb/src-revdb/revdb_include.h
+++ b/rpython/translator/revdb/src-revdb/revdb_include.h
@@ -1,21 +1,21 @@
 #include <string.h>
+#include "src/thread.h"
 
-/* By default, this makes an executable which supports both recording
-   and replaying.  It should help avoid troubles like using for
-   replaying an executable that is slightly different than the one
-   used for recording.  In theory you can compile with
-   -DRPY_RDB_REPLAY=0 or -DRPY_RDB_REPLAY=1 to get only one version
-   compiled for it, which should be slightly faster (not tested so
-   far).
-*/
+/************************************************************
+ ***  RevDB --- record and replay debugging               ***
+ ************************************************************/
+
 
 typedef struct {
 #ifndef RPY_RDB_REPLAY
     bool_t replay;
 #define RPY_RDB_REPLAY   rpy_revdb.replay
 #define RPY_RDB_DYNAMIC_REPLAY
+#else
+# error "explicit RPY_RDB_REPLAY: not really supported"
 #endif
     bool_t watch_enabled;
+    long lock;
     char *buf_p, *buf_limit, *buf_readend;
     uint64_t stop_point_seen, stop_point_break;
     uint64_t unique_id_seen, unique_id_break;
@@ -59,7 +59,19 @@
 #endif
 
 
-#define _RPY_REVDB_EMIT_RECORD(decl_e, variable)                        \
+/* Acquire/release the lock around EMIT_RECORD, because it may be
+   called without holding the GIL.  Note that we're always
+   single-threaded during replaying: the lock is only useful during
+   recording. */
+#define _RPY_REVDB_LOCK()                                               \
+    if (pypy_lock_test_and_set(&rpy_revdb.lock, 1) != 0)                \
+        rpy_reverse_db_lock_acquire();
+
+#define _RPY_REVDB_UNLOCK()                                             \
+    pypy_lock_release(&rpy_revdb.lock)
+
+
+#define _RPY_REVDB_EMIT_RECORD_L(decl_e, variable)                      \
         {                                                               \
             decl_e = variable;                                          \
             _RPY_REVDB_PRINT("write", _e);                              \
@@ -81,21 +93,28 @@
             variable = _e;                                              \
         }
 
-#define RPY_REVDB_EMIT(normal_code, decl_e, variable)                   \
+#define _RPY_REVDB_EMIT_L(normal_code, decl_e, variable, must_lock)     \
     if (!RPY_RDB_REPLAY) {                                              \
         normal_code                                                     \
-        _RPY_REVDB_EMIT_RECORD(decl_e, variable)                        \
+        if (must_lock) _RPY_REVDB_LOCK();                               \
+        _RPY_REVDB_EMIT_RECORD_L(decl_e, variable)                      \
+        if (must_lock) _RPY_REVDB_UNLOCK();                             \
     } else                                                              \
         _RPY_REVDB_EMIT_REPLAY(decl_e, variable)
 
+#define RPY_REVDB_EMIT(normal_code, decl_e, variable)                   \
+    _RPY_REVDB_EMIT_L(normal_code, decl_e, variable, 1)
+
 #define RPY_REVDB_EMIT_VOID(normal_code)                                \
     if (!RPY_RDB_REPLAY) { normal_code } else { }
 
 #define RPY_REVDB_CALL(call_code, decl_e, variable)                     \
     if (!RPY_RDB_REPLAY) {                                              \
         call_code                                                       \
-        _RPY_REVDB_EMIT_RECORD(unsigned char _e, 0xFC)                  \
-        _RPY_REVDB_EMIT_RECORD(decl_e, variable)                        \
+        _RPY_REVDB_LOCK();                                              \
+        _RPY_REVDB_EMIT_RECORD_L(unsigned char _e, 0xFC)                \
+        _RPY_REVDB_EMIT_RECORD_L(decl_e, variable)                      \
+        _RPY_REVDB_UNLOCK();                                            \
     } else {                                                            \
         unsigned char _re;                                              \
         _RPY_REVDB_EMIT_REPLAY(unsigned char _e, _re)                   \
@@ -107,7 +126,9 @@
 #define RPY_REVDB_CALL_VOID(call_code)                                  \
     if (!RPY_RDB_REPLAY) {                                              \
         call_code                                                       \
-        _RPY_REVDB_EMIT_RECORD(unsigned char _e, 0xFC)                  \
+        _RPY_REVDB_LOCK();                                              \
+        _RPY_REVDB_EMIT_RECORD_L(unsigned char _e, 0xFC)                \
+        _RPY_REVDB_UNLOCK();                                            \
     }                                                                   \
     else {                                                              \
         unsigned char _re;                                              \
@@ -174,7 +195,7 @@
 #define RPY_REVDB_CAST_PTR_TO_INT(obj)   (((struct pypy_header0 *)obj)->h_uid)
 
 
-RPY_EXTERN void rpy_reverse_db_flush(void);
+RPY_EXTERN void rpy_reverse_db_flush(void);  /* must be called with the lock */
 RPY_EXTERN void rpy_reverse_db_fetch(const char *file, int line);
 RPY_EXTERN void rpy_reverse_db_stop_point(long place);
 RPY_EXTERN void rpy_reverse_db_send_answer(int cmd, int64_t arg1, int64_t arg2,
@@ -193,5 +214,6 @@
 RPY_EXTERN void rpy_reverse_db_call_destructor(void *obj);
 RPY_EXTERN void rpy_reverse_db_invoke_callback(unsigned char);
 RPY_EXTERN void rpy_reverse_db_callback_loc(int);
+RPY_EXTERN void rpy_reverse_db_lock_acquire(void);
 
 /* ------------------------------------------------------------ */


More information about the pypy-commit mailing list