Browse Source

fix some race conditions

David Rose 16 years ago
parent
commit
2c33bc2bd3

+ 1 - 1
direct/src/plugin/p3dConditionVar.cxx

@@ -139,7 +139,7 @@ wait(double timeout) {
   ts.tv_nsec += (int)((timeout - seconds) * 1000000.0);
   ts.tv_nsec += (int)((timeout - seconds) * 1000000.0);
 
 
   int result = pthread_cond_timedwait(&_cvar, &_lock, &ts);
   int result = pthread_cond_timedwait(&_cvar, &_lock, &ts);
-  assert(result == 0);
+  assert(result == 0 || errno == ETIMEDOUT);
 
 
 #endif  // _WIN32
 #endif  // _WIN32
 }
 }

+ 3 - 0
direct/src/plugin/p3dPythonRun.cxx

@@ -237,6 +237,9 @@ handle_command(TiXmlDocument *doc) {
         // does make debugging the logs a bit easier.
         // does make debugging the logs a bit easier.
         _next_sent_id = _session_id * 1000;
         _next_sent_id = _session_id * 1000;
 
 
+        PyObject *obj = PyObject_CallMethod(_runner, (char*)"setSessionId", (char *)"i", _session_id);
+        Py_XDECREF(obj);
+
       } else if (strcmp(cmd, "start_instance") == 0) {
       } else if (strcmp(cmd, "start_instance") == 0) {
         assert(!needs_response);
         assert(!needs_response);
         TiXmlElement *xinstance = xcommand->FirstChildElement("instance");
         TiXmlElement *xinstance = xcommand->FirstChildElement("instance");

+ 22 - 33
direct/src/plugin/p3dSession.cxx

@@ -44,8 +44,6 @@ P3DSession(P3DInstance *inst) {
   _python_version = inst->get_python_version();
   _python_version = inst->get_python_version();
 
 
   _p3dpython_running = false;
   _p3dpython_running = false;
-  _response = NULL;
-  _got_response_id = -1;
 
 
   _started_read_thread = false;
   _started_read_thread = false;
   _read_thread_continue = false;
   _read_thread_continue = false;
@@ -275,17 +273,8 @@ command_and_response(TiXmlDocument *command) {
   // only one thread will be waiting at a time.
   // only one thread will be waiting at a time.
   nout << "waiting for response " << response_id << "\n" << flush;
   nout << "waiting for response " << response_id << "\n" << flush;
   _response_ready.acquire();
   _response_ready.acquire();
-  while (_response == NULL || _got_response_id != response_id) {
-    if (_response != NULL) {
-      // This is a bogus response.  Since we're the only thread waiting,
-      // it follows that no one is waiting for this response, so we can
-      // throw it away.
-      nout << "Discarding bogus response: " << *_response << "\n" << flush;
-      delete _response;
-      _response = NULL;
-      _got_response_id = -1;
-    }
-
+  Responses::iterator ri = _responses.find(response_id);
+  while (ri == _responses.end()) {
     if (!_p3dpython_running) {
     if (!_p3dpython_running) {
       // Hmm, looks like Python has gone away.
       // Hmm, looks like Python has gone away.
 
 
@@ -299,11 +288,23 @@ command_and_response(TiXmlDocument *command) {
     // recursive script requests.  (The child process might have to
     // recursive script requests.  (The child process might have to
     // wait for us to process some of these before it can fulfill the
     // wait for us to process some of these before it can fulfill the
     // command we're actually waiting for.)
     // command we're actually waiting for.)
+
+    // Release the mutex while we do this, so we can safely call back
+    // in recursively.
+    _response_ready.release();
     Instances::iterator ii;
     Instances::iterator ii;
     for (ii = _instances.begin(); ii != _instances.end(); ++ii) {
     for (ii = _instances.begin(); ii != _instances.end(); ++ii) {
       P3DInstance *inst = (*ii).second;
       P3DInstance *inst = (*ii).second;
       inst->bake_requests();
       inst->bake_requests();
     }
     }
+    _response_ready.acquire();
+
+    ri = _responses.find(response_id);
+    if (ri != _responses.end()) {
+      // We got the response we were waiting for while we had the
+      // mutex unlocked.
+      break;
+    }
 
 
 #ifdef _WIN32
 #ifdef _WIN32
     // Make sure we process the Windows event loop while we're
     // Make sure we process the Windows event loop while we're
@@ -314,23 +315,19 @@ command_and_response(TiXmlDocument *command) {
     // A single PeekMessage() seems to be sufficient.
     // A single PeekMessage() seems to be sufficient.
     MSG msg;
     MSG msg;
     PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE | PM_NOYIELD);
     PeekMessage(&msg, NULL, 0, 0, PM_NOREMOVE | PM_NOYIELD);
+#endif  // _WIN32
 
 
     // We wait with a timeout, so we can go back and spin the event
     // We wait with a timeout, so we can go back and spin the event
     // loop some more.
     // loop some more.
     _response_ready.wait(0.1);
     _response_ready.wait(0.1);
     nout << "." << flush;
     nout << "." << flush;
-#else  // _WIN32
-    
-    // On non-Windows platforms, we can just wait indefinitely.
-    _response_ready.wait();
-#endif  // _WIN32
+
+    ri = _responses.find(response_id);
   }
   }
   // When we exit the loop, we've found the desired response.
   // When we exit the loop, we've found the desired response.
-  nout << "got response: " << *_response << "\n" << flush;
-
-  TiXmlDocument *response = _response;
-  _response = NULL;
-  _got_response_id = -1;
+  TiXmlDocument *response = (*ri).second;
+  nout << "got response: " << *response << "\n" << flush;
+  _responses.erase(ri);
 
 
   _response_ready.release();
   _response_ready.release();
 
 
@@ -747,16 +744,8 @@ rt_handle_request(TiXmlDocument *doc) {
       // This is a response to a previous command-and-response.  Send
       // This is a response to a previous command-and-response.  Send
       // it to the parent thread.
       // it to the parent thread.
       _response_ready.acquire();
       _response_ready.acquire();
-      if (_response != NULL) {
-        // Hey, there's already a response there.  Since there's only
-        // one thread waiting at a time on the command-response cycle,
-        // this must be a bogus response that never got picked up.
-        // Discard it.
-        nout << "Discarding bogus response: " << *_response << "\n";
-        delete _response;
-      }
-      _response = doc;
-      _got_response_id = response_id;
+      bool inserted = _responses.insert(Responses::value_type(response_id, doc)).second;
+      assert(inserted);
       _response_ready.notify();
       _response_ready.notify();
       _response_ready.release();
       _response_ready.release();
       return;
       return;

+ 3 - 3
direct/src/plugin/p3dSession.h

@@ -132,9 +132,9 @@ private:
 #endif
 #endif
   bool _p3dpython_running;
   bool _p3dpython_running;
 
 
-  // The _response_ready mutex protects this pointer.
-  TiXmlDocument *_response;
-  int _got_response_id;
+  // The _response_ready mutex protects this structure.
+  typedef map<int, TiXmlDocument *> Responses;
+  Responses _responses;
   P3DConditionVar _response_ready;
   P3DConditionVar _response_ready;
 
 
   // The remaining members are manipulated by or for the read thread.
   // The remaining members are manipulated by or for the read thread.

+ 12 - 3
direct/src/showutil/runp3d.py

@@ -51,7 +51,8 @@ class ScriptAttributes:
 class AppRunner(DirectObject):
 class AppRunner(DirectObject):
     def __init__(self):
     def __init__(self):
         DirectObject.__init__(self)
         DirectObject.__init__(self)
-        
+
+        self.sessionId = 0
         self.packedAppEnvironmentInitialized = False
         self.packedAppEnvironmentInitialized = False
         self.gotWindow = False
         self.gotWindow = False
         self.gotP3DFilename = False
         self.gotP3DFilename = False
@@ -94,6 +95,11 @@ class AppRunner(DirectObject):
         if AppRunnerGlobal.appRunner is None:
         if AppRunnerGlobal.appRunner is None:
             AppRunnerGlobal.appRunner = self
             AppRunnerGlobal.appRunner = self
 
 
+    def setSessionId(self, sessionId):
+        """ This message should come in at startup. """
+        self.sessionId = sessionId
+        self.nextScriptId = self.sessionId * 1000 + 10000
+
     def initPackedAppEnvironment(self):
     def initPackedAppEnvironment(self):
         """ This function sets up the Python environment suitably for
         """ This function sets up the Python environment suitably for
         running a packed app.  It should only run once in any given
         running a packed app.  It should only run once in any given
@@ -272,6 +278,9 @@ class AppRunner(DirectObject):
         settings, for future windows; or applies them directly to the
         settings, for future windows; or applies them directly to the
         main window if the window has already been opened. """
         main window if the window has already been opened. """
 
 
+        print "session %s, nextScriptId = %s" % (self.sessionId, self.nextScriptId)
+
+
         print "setupWindow %s, %s, %s, %s, %s, %s, %s" % (windowType, x, y, width, height, parent, subprocessWindow)
         print "setupWindow %s, %s, %s, %s, %s, %s, %s" % (windowType, x, y, width, height, parent, subprocessWindow)
 
 
         if self.started and base.win:
         if self.started and base.win:
@@ -400,7 +409,7 @@ class AppRunner(DirectObject):
         waiting for the browser to process the request.
         waiting for the browser to process the request.
         """
         """
         uniqueId = self.nextScriptId
         uniqueId = self.nextScriptId
-        self.nextScriptId += 1
+        self.nextScriptId = (self.nextScriptId + 1) % 0xffffffff
         self.sendRequest('script', operation, object,
         self.sendRequest('script', operation, object,
                          propertyName, value, needsResponse, uniqueId)
                          propertyName, value, needsResponse, uniqueId)
 
 
@@ -677,7 +686,7 @@ class MethodWrapper:
         raise AttributeError(name)
         raise AttributeError(name)
 
 
 if __name__ == '__main__':
 if __name__ == '__main__':
-    runner = AppRunner()
+    runner = AppRunner(0)
     runner.gotWindow = True
     runner.gotWindow = True
     try:
     try:
         runner.setP3DFilename(*runner.parseSysArgs())
         runner.setP3DFilename(*runner.parseSysArgs())