Merge lp:~zorba-coders/zorba/debugger_stop_command into lp:zorba

[Gabriel Petrovay]

Implemented the stop command.
Fixed termination on Windows.
TODO: still a problem on Windows when quitting the debugger client: a zorba store is shut down and in the store destructor a lock hangs the execution of xqdb forever. Not even Ctrl-C helps (probably because everything is called from the exit function in main.cpp)

Implemented the stop command.
Fixed termination on Windows.
TODO: still a problem on Windows when quitting the debugger client: a zorba store is shut down and in the store destructor a lock hangs the execution of xqdb forever. Not even Ctrl-C helps (probably because everything is called from the exit function in main.cpp)

[Gabriel Petrovay (gabipetrovay)]

review: Approve

[David Graf (davidagraf)]

It works. But during testing this feature, I managed to produce an Assertion that has nothing to do with this change. I file a bug for it.

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/debugger_stop_command-2012-01-11T13-10-06.289Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job debugger_stop_command-2012-01-11T13-10-06.289Z is finished. The final status was:

All tests succeeded!

=== modified file 'bin/debugger/command_line_handler.cpp'
--- bin/debugger/command_line_handler.cpp	2012-01-03 12:10:06 +0000
+++ bin/debugger/command_line_handler.cpp	2012-01-10 20:09:25 +0000
@@ -116,7 +116,7 @@
       }
     }
   }
-  theWaitFor = theClient->stop();
+  theWaitFor = theClient->stop(true);
   theClient->quit();
   theQuit = true;
 }
@@ -131,6 +131,13 @@
 
 template<>
 void
+CommandLineHandler::handle<Stop>(ZORBA_TR1_NS::tuple<> &t)
+{
+  theWaitFor = theClient->stop(false);
+}
+
+template<>
+void
 CommandLineHandler::handle<BreakpointSet>(std::tr1::tuple<bstring, bstring, bint> &aTuple)
 {
   DebuggerClient::BreakpointType lType = DebuggerClient::Line;
@@ -271,6 +278,9 @@
   // DBGP: run
   *theCommandLine << createCommand<Run>(TUPLE(), "run", *this, "Run the query");
 
+  // DBGP: stop
+  *theCommandLine << createCommand<Stop>(TUPLE(), "stop", *this, "Stop the query");
+
   // DBGP: breakpoint_set
   {
     std::set<std::string> lAliases;

=== modified file 'bin/debugger/command_line_handler.h'
--- bin/debugger/command_line_handler.h	2012-01-03 12:10:06 +0000
+++ bin/debugger/command_line_handler.h	2012-01-10 20:09:25 +0000
@@ -35,6 +35,7 @@
     Variables,
     Quit,
     Run,
+    Stop,
     BreakpointSet,
     BreakpointGet,
     BreakpointRemove,
@@ -132,10 +133,13 @@
 
 template<>
 void CommandLineHandler::handle<Run> (ZORBA_TR1_NS::tuple<> &t);
-  
+
+template<>
+void CommandLineHandler::handle<Stop> (ZORBA_TR1_NS::tuple<> &t);
+
 template<>
 void CommandLineHandler::handle<StackDepth> (ZORBA_TR1_NS::tuple<>& t);
-  
+
 template<>
 void CommandLineHandler::handle<BreakpointList> (ZORBA_TR1_NS::tuple<>& aTuple);
   

=== modified file 'bin/debugger/event_handler.cpp'
--- bin/debugger/event_handler.cpp	2011-12-21 14:40:33 +0000
+++ bin/debugger/event_handler.cpp	2012-01-10 20:09:25 +0000
@@ -91,8 +91,7 @@
   } catch (ZorbaException& e) {
     std::cerr << "FATAL: could not execute query: " << std::endl;
     std::cerr << e << std::endl;
-    std::cerr << "This is a bug, please report to [e-mail]" << std::endl;
-    //theContinueProducer.produce(false);
+    std::cerr << "This is a bug, please report it at http://bugs.launchpad.net/zorba" << std::endl;
   }
 }
   

=== modified file 'bin/debugger/main.cpp'
--- bin/debugger/main.cpp	2012-01-05 13:19:14 +0000
+++ bin/debugger/main.cpp	2012-01-10 20:09:25 +0000
@@ -78,9 +78,6 @@
 };
 
 
-XqdbClient* theClient;
-
-
 void
 onExitProcess(ExitCode aExitCode) {
   //if (aExitCode != -1) {
@@ -89,12 +86,12 @@
   std::cout << "Terminating debugger client."<< std::endl;
   // TODO: and the memory?
 
-  delete theClient;
   exit(aExitCode);
 }
 
+
 int
-startZorba(std::string& aExec, std::vector<std::string>& aArgs) 
+startZorba(std::string& aExec, std::vector<std::string>& aArgs, std::auto_ptr<ProcessListener>& aProcessListener) 
 {
 #ifdef WIN32
   // **************************
@@ -143,7 +140,7 @@
 
   if (lResult) {
     // Watch the process
-    ProcessListener* lPl = new ProcessListener(piProcessInfo.hProcess, &onExitProcess);
+    aProcessListener.reset(new ProcessListener(piProcessInfo.dwProcessId, &onExitProcess));
   }
   else {
     // CreateProcess failed
@@ -215,7 +212,7 @@
     }
     
     // Watch the process
-    new ProcessListener(pID, &onExitProcess);
+    aProcessListener.reset(new ProcessListener(pID, &onExitProcess));
 
     return 0;
   }
@@ -357,8 +354,11 @@
     // **************************************************************************
     // start a zorba
 
+    // This is a process listener used to watch the Zorba process termination.
+    std::auto_ptr<ProcessListener> lProcessListener;
+
     if (!lStandalone) {
-      int lResult = startZorba(lZorbaExec, lZorbaArgs);
+      int lResult = startZorba(lZorbaExec, lZorbaArgs, lProcessListener);
       if (lResult) {
         return lResult;
       }
@@ -369,13 +369,10 @@
     // **************************************************************************
     // start the debugger command line
 
-    theClient =  new XqdbClient(lPort);
+    std::auto_ptr<XqdbClient> theClient(new XqdbClient(lPort));
     theClient->start();
 
-    delete theClient;
-
   } catch (...) {
-    delete theClient;
     return -1;
   }
 

=== modified file 'bin/debugger/process_listener.cpp'
--- bin/debugger/process_listener.cpp	2012-01-05 13:19:14 +0000
+++ bin/debugger/process_listener.cpp	2012-01-10 20:09:25 +0000
@@ -62,16 +62,20 @@
   ProcessId lPid = lThis->getProcessID();
 
 #ifdef WIN32
-  // wait for the process to exit
-  WaitForSingleObject(lPid, INFINITE);
-
-  // find out the process exit code if possible
-  if (!GetExitCodeProcess(lThis->getProcessID(), &lExitCode)) {
-    lExitCode = -1;
+  HANDLE lProcessHandle = OpenProcess(SYNCHRONIZE, false, lPid);
+  if (lProcessHandle != NULL) {
+    // wait for the process to exit
+    DWORD lResult = WaitForSingleObject(lProcessHandle, INFINITE);
+ 
+    // find out the process exit code if possible
+    if (!GetExitCodeProcess(lProcessHandle, &lExitCode)) {
+      lExitCode = -1;
+    }
+    DWORD dw = GetLastError();
+ 
+    // wait a little for zorba to dump the garbage
+    Sleep(1000);
   }
-
-  // wait a little for zorba to dump the garbage
-  Sleep(1000);
 #else
   int lChildExitStatus;
 

=== modified file 'bin/debugger/process_listener.h'
--- bin/debugger/process_listener.h	2012-01-05 13:19:14 +0000
+++ bin/debugger/process_listener.h	2012-01-10 20:09:25 +0000
@@ -29,7 +29,7 @@
 #else
 # include <windows.h>
   typedef DWORD ThreadId;
-  typedef HANDLE ProcessId;
+  typedef DWORD ProcessId;
   typedef DWORD ExitCode;
 # define ZORBA_THREAD_RETURN DWORD WINAPI
 #endif

=== modified file 'include/zorba/debugger_client.h'
--- include/zorba/debugger_client.h	2011-12-21 14:40:33 +0000
+++ include/zorba/debugger_client.h	2012-01-10 20:09:25 +0000
@@ -273,9 +273,13 @@
      * This command tells the debug engine, that it should
      * break the execution at the next point possible.
      *
+     * @param withQuit This is a Zorba extension of the DBGP protocol that
+     *    controls if the client should terminate execution and quit (true)
+     *    or only terminate execution but not quit (false). This is used
+     *    by command line clients that implement multiple query runs.
      * @return The id of this request
      */
-    virtual std::size_t stop() = 0;
+    virtual std::size_t stop(bool withQuit) = 0;
     
     /**
      * @brief Send the detach command to the debug engine.

=== modified file 'src/debugger/debugger_clientimpl.cpp'
--- src/debugger/debugger_clientimpl.cpp	2011-12-21 14:40:33 +0000
+++ src/debugger/debugger_clientimpl.cpp	2012-01-10 20:09:25 +0000
@@ -213,10 +213,14 @@
 }
   
 std::size_t
-DebuggerClientImpl::stop()
+DebuggerClientImpl::stop(bool withQuit)
 {
   std::size_t id = ++theLastId;
-  *theOutStream << "stop -i " << id << '\0';
+  *theOutStream << "stop -i " << id;
+  if (!withQuit) {
+    *theOutStream << " -z 1";
+  }
+  *theOutStream << '\0';
   theOutStream->flush();
   return id;
 }

=== modified file 'src/debugger/debugger_clientimpl.h'
--- src/debugger/debugger_clientimpl.h	2011-12-21 14:40:33 +0000
+++ src/debugger/debugger_clientimpl.h	2012-01-10 20:09:25 +0000
@@ -57,7 +57,7 @@
     virtual std::size_t step_into();
     virtual std::size_t step_out();
     virtual std::size_t step_over();
-    virtual std::size_t stop();
+    virtual std::size_t stop(bool withQuit = true);
     virtual std::size_t detach();
     virtual std::size_t breakpoint_set(BreakpointType aType,
                                        bool aEnabled = true,

=== modified file 'src/debugger/debugger_communicator.cpp'
--- src/debugger/debugger_communicator.cpp	2011-12-21 14:40:33 +0000
+++ src/debugger/debugger_communicator.cpp	2012-01-10 20:09:25 +0000
@@ -74,13 +74,17 @@
   }
 }
 
+#ifdef NDEBUG
+# define TIMEOUT 6
+#else
+# define TIMEOUT 60
+#endif
+
 void
 DebuggerCommunicator::connect()
 {
-	for (int i = 0; i < 5 && !theSocket; i++)
-  {
-		try
-    {
+	for (int i = 0; i < TIMEOUT && !theSocket; i++) {
+		try {
 			// Connect to the client on the given host and port
       std::auto_ptr<TCPSocket> lSocket(new TCPSocket(theHost, thePort));
       theSocket = lSocket.release();
@@ -91,10 +95,9 @@
       theResponseQueue = new ResponseQueue(theCommunicatorOutStream);
       theResponseQueue->start();
 		}
-    catch (DebuggerSocketException& /* e */)
-    {
+    catch (DebuggerSocketException& /* e */) {
       // Wait one second before trying to reconnect
-      msleep(100);
+      msleep(1000);
 		}
   }
 }

=== modified file 'src/debugger/debugger_runtime.cpp'
--- src/debugger/debugger_runtime.cpp	2012-01-03 12:10:06 +0000
+++ src/debugger/debugger_runtime.cpp	2012-01-10 20:09:25 +0000
@@ -67,7 +67,6 @@
     theCommunicator(communicator),
     theWrapper(theQuery->generateWrapper()),
     theExecStatus(QUERY_IDLE),
-    theNotSendTerminateEvent(false),
     thePlanIsOpen(false),
     theSerializer(0),
     theItemHandler(aHandler),
@@ -89,6 +88,14 @@
   theWrapper->open();
   thePlanIsOpen = true;
   runQuery();
+
+  std::stringstream lResult;
+  lResult << "<response command=\"" << theLastContinuationCommand.second << "\" "
+    << "status=\"stopping\" "
+    << "reason=\"ok\" "
+    << "transaction_id=\"" << theLastContinuationCommand.first << "\">"
+    << "</response>";
+  theCommunicator->send(lResult.str());
 }
 
 void
@@ -107,7 +114,6 @@
   theWrapper = theQuery->generateWrapper();
   thePlanIsOpen = false;
   theExecStatus = QUERY_IDLE;
-  theNotSendTerminateEvent = false;
   reset();
 }
 
@@ -324,15 +330,6 @@
 {
   AutoLock lLock(theLock, Lock::WRITE);
   theExecStatus = QUERY_TERMINATED;
-
-  std::stringstream lResult;
-  lResult << "<response command=\"" << theLastContinuationCommand.second << "\" "
-    << "status=\"stopping\" "
-    << "reason=\"ok\" "
-    << "transaction_id=\"" << theLastContinuationCommand.first << "\">"
-    << "</response>";
-  theCommunicator->send(lResult.str());
-  // TODO: something more here?
 }
 
 
@@ -503,13 +500,6 @@
 }
 
 
-void
-DebuggerRuntime::setNotSendTerminateEvent()
-{
-  theNotSendTerminateEvent = true;
-}
-
-
 std::list<std::pair<zstring, zstring> >
 DebuggerRuntime::eval(zstring& aExpr)
 {

=== modified file 'src/debugger/debugger_runtime.h'
--- src/debugger/debugger_runtime.h	2012-01-03 12:10:06 +0000
+++ src/debugger/debugger_runtime.h	2012-01-10 20:09:25 +0000
@@ -67,9 +67,6 @@
   public:
 
     void
-    setNotSendTerminateEvent();
-
-    void
     resetRuntime();
 
     ExecutionStatus
@@ -173,7 +170,6 @@
     ExecutionStatus                   theExecStatus;
     mutable Lock                      theLock;
     std::set<DebugIterator*>          theBreakpoints;
-    bool                              theNotSendTerminateEvent;
     bool                              thePlanIsOpen;
     serializer*                       theSerializer;
     itemHandler                       theItemHandler;

=== modified file 'src/debugger/debugger_server.cpp'
--- src/debugger/debugger_server.cpp	2012-01-03 12:10:06 +0000
+++ src/debugger/debugger_server.cpp	2012-01-10 20:09:25 +0000
@@ -78,6 +78,8 @@
   init();
 
   std::string lCommand;
+  std::string lCommandName;
+  int lTransactionID = 0;
 
   while (!theStopping &&
       theRuntime->getExecutionStatus() != QUERY_DETACHED) {
@@ -86,6 +88,9 @@
     theCommunicator->receive(lCommand);
     DebuggerCommand lCmd = DebuggerCommand(lCommand);
 
+    lCommandName = lCmd.getName();
+    lCmd.getArg("i", lTransactionID);
+
     if (theRuntime->getExecutionStatus() == QUERY_TERMINATED) {
       // clone the existing runtime
       DebuggerRuntime* lNewRuntime = theRuntime->clone();
@@ -108,7 +113,16 @@
   }
 
   theRuntime->terminate();
-  theRuntime->resetRuntime();
+
+  std::stringstream lResult;
+  lResult << "<response command=\"" << lCommandName << "\" "
+    << "status=\"stopped\" "
+    << "reason=\"ok\" "
+    << "transaction_id=\"" << lTransactionID << "\">"
+    << "</response>";
+  theCommunicator->send(lResult.str());
+
+  //theRuntime->resetRuntime();
   theRuntime->join();
 
   return true;
@@ -473,10 +487,18 @@
 
       } else if (aCommand.getName() == "stop") {
         theRuntime->setLastContinuationCommand(lTransactionID, aCommand.getName());
-        theStopping = true;
+        // sending the zorba extensions flag, the debugger server will not terminate
+        // when the stop command is sent. This way the zorba debugger client can
+        // perform multiple execution of the same query even when the user terminates
+        // the execution using the stop command.
+        // NOTE: theStopping is controlling the main debugger server loop
+        if (!lZorbaExtensions) {
+          theStopping = true;
+        }
 
-        lResponse << "reason=\"ok\" status=\"stopped\" ";
+        lResponse << "status=\"stopping\" reason=\"ok\"";
         lResponse << ">";
+
         theRuntime->terminateRuntime();
 
       } else if (aCommand.getName() == "stack_depth") {
@@ -531,8 +553,7 @@
         theRuntime->stepOver();
         return "";
       } else if (aCommand.getName() == "step_out") {
-        ExecutionStatus lStatus = theRuntime->getExecutionStatus();
-        if (lStatus != QUERY_SUSPENDED) {
+        if (theRuntime->getExecutionStatus() != QUERY_SUSPENDED) {
           return buildErrorResponse(lTransactionID, lCmdName, 6, "Can not step out since the execution is not started.");
         }
         theRuntime->setLastContinuationCommand(lTransactionID, aCommand.getName());