[PATCH] 670439 - Nemiver doesn't handle well multi-threaded apps

[Dodji Seketeli <dodji seketeli org>]

Hello,

As the audit trail of the bug says, Nemiver fails to choose the right
thread to look for local variables in a multi-threaded environment.

What happens is that it always asks GDB for variables in the 'current'
thread.  But the problem is, the current thread was not being correctly
set whenever Nemiver receives out-of-band MI records from GDB saying
that the current thread changed.

Fixed thus in the GDB/MI parser by making sure we correctly set the
current thread id.

Tested and applying to the master branch.

	* src/dbgengine/nmv-dbg-common.h
	(OutOfBandRecord::m_thread_selected): New attribute.
	(OutOfBandRecord::thread_selected): New accessor.
	(OutOfBandRecord::clear): Clear the new m_thread_selected
	attribute.
	* src/dbgengine/nmv-gdbmi-parser.cc
	(GDBMIParser::parse_out_of_band_record): Make the thread as
	selected when we get an async 'thread-selected' output.
	(OnThreadSelectedHandler::can_handle): Use the new
	OutOfBandRecord::thread_selected to detect that we have an out of
	band record (async) that says a new thread has been selected and
	thus notify the world that we have a new 'current thread id' that
	is set.
	* tests/test-threads.cc: New test file, using the existing
	threads.cc as a program to debug and inspect.
	* tests/Makefile.am: Integrate test-threads.cc into the build
	system.
	* threads.cc (thread_func): Don't output messages.  Add a tid
	variable so that the test-threads.cc can inspect it.
---
 src/dbgengine/nmv-dbg-common.h    |   12 +++--
 src/dbgengine/nmv-gdb-engine.cc   |    8 +--
 src/dbgengine/nmv-gdbmi-parser.cc |    1 +
 tests/Makefile.am                 |    8 +++-
 tests/test-threads.cc             |  103 +++++++++++++++++++++++++++++++++++++
 tests/threads.cc                  |    5 +--
 6 files changed, 123 insertions(+), 14 deletions(-)
 create mode 100644 tests/test-threads.cc

diff --git a/src/dbgengine/nmv-dbg-common.h b/src/dbgengine/nmv-dbg-common.h
index 53e4c5a..bf62626 100644
--- a/src/dbgengine/nmv-dbg-common.h
+++ b/src/dbgengine/nmv-dbg-common.h
@@ -256,10 +256,9 @@ public:
 
     /// \brief the out of band record we got from GDB.
     ///
-    ///Out of band record is either
-    ///a set of messages sent by gdb
-    ///to tell us about the reason why the target has stopped,
-    ///or, a stream record.
+    /// An Out of band record is an aynchronous record that is either
+    /// a set of messages sent by the debugger to tell us about the reason why
+    /// the target has changed state.
     class OutOfBandRecord {
     public:
 
@@ -271,6 +270,7 @@ public:
 	bool m_is_running;
         IDebugger::StopReason m_stop_reason;
         bool m_has_frame;
+        bool m_thread_selected;
         IDebugger::Frame m_frame;
         long m_breakpoint_number;
         long m_thread_id;
@@ -363,6 +363,9 @@ public:
         long breakpoint_number () const {return m_breakpoint_number;}
         void breakpoint_number (long a_in) {m_breakpoint_number = a_in;}
 
+        bool thread_selected () const {return m_thread_selected;}
+        void thread_selected (bool a_in) {m_thread_selected = a_in;}
+
         long thread_id () const {return m_thread_id;}
         void thread_id (long a_in) {m_thread_id = a_in;}
 
@@ -384,6 +387,7 @@ public:
             m_is_running = false;
             m_stop_reason = IDebugger::UNDEFINED_REASON;
             m_has_frame = false;
+            m_thread_selected = false;
             m_frame.clear ();
             m_breakpoint_number = 0;
             m_thread_id = -1;
diff --git a/src/dbgengine/nmv-gdb-engine.cc b/src/dbgengine/nmv-gdb-engine.cc
index 2bb076f..e8f62e2 100644
--- a/src/dbgengine/nmv-gdb-engine.cc
+++ b/src/dbgengine/nmv-gdb-engine.cc
@@ -1758,12 +1758,10 @@ struct OnThreadSelectedHandler : OutputHandler {
             for (it = a_in.output ().out_of_band_records ().begin ();
                  it != a_in.output ().out_of_band_records ().end ();
                  ++it) {
-                if (it->thread_id ()) {
+                if (it->thread_selected ()) {
                     thread_id = it->thread_id ();
-                    return false; // GDB is broken currently
-                                  // for this. So return false
-                                  // for now. We'll be able to return
-                                  // true later.;
+                    THROW_IF_FAIL (thread_id > 0);
+                    return true;
                 }
             }
         }
diff --git a/src/dbgengine/nmv-gdbmi-parser.cc b/src/dbgengine/nmv-gdbmi-parser.cc
index 0076e92..085781b 100644
--- a/src/dbgengine/nmv-gdbmi-parser.cc
+++ b/src/dbgengine/nmv-gdbmi-parser.cc
@@ -1833,6 +1833,7 @@ GDBMIParser::parse_out_of_band_record (UString::size_type a_from,
             return false;
         }
         record.thread_id (thread_id);
+        record.thread_selected (true);
         goto end;
     }
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index c9bb30e..39522a4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -17,7 +17,8 @@ runtestlocalvarslist runtestcpplexer \
 runtestcppparser runtestvarpathexpr \
 runtestlibtoolwrapperdetection \
 runtesttypes runtestdisassemble \
-runtestvariableformat runtestprettyprint
+runtestvariableformat runtestprettyprint \
+runtestthreads
 
 else
 
@@ -85,6 +86,11 @@ runtestprettyprint_LDADD=@NEMIVERCOMMON_LIBS@ \
 $(top_builddir)/src/common/libnemivercommon.la \
 $(top_builddir)/src/dbgengine/libdebuggerutils.la
 
+runtestthreads_SOURCES=$(h)/test-threads.cc
+runtestthreads_LDADD=@NEMIVERCOMMON_LIBS@ \
+$(top_builddir)/src/common/libnemivercommon.la \
+$(top_builddir)/src/dbgengine/libdebuggerutils.la
+
 #runtestoverloads_SOURCES=$(h)/test-overloads.cc
 #runtestoverloads_LDADD=@NEMIVERCOMMON_LIBS@ \
 #$(top_builddir)/src/common/libnemivercommon.la
diff --git a/tests/test-threads.cc b/tests/test-threads.cc
new file mode 100644
index 0000000..9decf29
--- /dev/null
+++ b/tests/test-threads.cc
@@ -0,0 +1,103 @@
+#include "config.h"
+#include <iostream>
+#include <boost/test/minimal.hpp>
+#include "common/nmv-initializer.h"
+#include "common/nmv-safe-ptr-utils.h"
+#include "common/nmv-exception.h"
+#include "nmv-debugger-utils.h"
+
+using namespace nemiver;
+using namespace nemiver::common;
+
+static Glib::RefPtr<Glib::MainLoop> loop =
+    Glib::MainLoop::create (Glib::MainContext::get_default ());
+
+static unsigned num_threads = 0;
+
+static void
+on_engine_died_signal ()
+{
+    MESSAGE ("engine died");
+    loop->quit ();
+}
+
+static void
+on_program_finished_signal ()
+{
+    MESSAGE ("program finished");
+    BOOST_REQUIRE (num_threads == 5);
+    loop->quit ();
+}
+
+static void
+on_stopped_signal (IDebugger::StopReason a_reason,
+                   bool /*a_has_frame*/,
+                   const IDebugger::Frame &/*a_frame*/,
+                   int /*a_thread_id*/,
+                   int /*a_bp_num*/,
+                   const UString &/*a_cookie*/,
+                   IDebuggerSafePtr &a_debugger)
+{
+    if (a_reason == IDebugger::BREAKPOINT_HIT)
+        a_debugger->print_variable_value ("tid");
+
+    a_debugger->do_continue ();
+}
+
+static void
+on_variable_value_signal (const UString&,
+                          const IDebugger::VariableSafePtr a_expr,
+                          const UString&)
+{
+    if (a_expr->name () == "tid")
+        ++num_threads;
+}
+
+NEMIVER_API int
+test_main (int, char *[])
+{
+    NEMIVER_TRY;
+
+    Initializer::do_init ();
+
+    THROW_IF_FAIL (loop);
+
+    IDebuggerSafePtr debugger =
+        debugger_utils::load_debugger_iface_with_confmgr ();
+
+    debugger->set_event_loop_context (loop->get_context ());
+
+    //*****************************
+    //<connect to IDebugger events>
+    //*****************************
+
+    debugger->engine_died_signal ().connect (&on_engine_died_signal);
+
+    debugger->program_finished_signal ().connect
+        (&on_program_finished_signal);
+
+    debugger->variable_value_signal ().connect
+        (&on_variable_value_signal);
+
+    debugger->stopped_signal ().connect
+        (sigc::bind (&on_stopped_signal, debugger));
+
+    //*****************************
+    //</connect to IDebugger events>
+    //*****************************
+
+    std::vector<UString> args, source_search_dir;
+    debugger->enable_pretty_printing (false);
+    source_search_dir.push_back (".");
+    debugger->load_program ("threads", args, ".",
+                            source_search_dir, "",
+                            false);
+    debugger->set_breakpoint ("threads.cc", 32);
+
+    debugger->run ();
+    loop->run ();
+
+    NEMIVER_CATCH_NOX;
+
+    return 0;
+}
diff --git a/tests/threads.cc b/tests/threads.cc
index 1d19e65..ed035eb 100644
--- a/tests/threads.cc
+++ b/tests/threads.cc
@@ -28,10 +28,7 @@ thread_func (void *arg)
 {
     assert (arg != 0);
     thread_info *ti = static_cast<thread_info*> (arg);
-    for (int i = 0; i < NUM_ITER; ++i) {
-        cout << "thread " << ti->tnum << ":iter:" << i << "\n";
-    }
-    cout << flush;
+    __attribute__((unused)) int tid = ti->tid;
     return NULL;
 }
 
-- 
		Dodji