better object_id pointer passing

direct/src/plugin/p3dPythonObject.cxx

@@ -211,11 +211,14 @@ call(const string &method_name, P3D_object *params[], int num_params) const {
   TiXmlElement *xcommand = new TiXmlElement("command");
   xcommand->SetAttribute("cmd", "pyobj");
   xcommand->SetAttribute("op", "call");
-  xcommand->SetAttribute("object_id", _object_id);
   if (!method_name.empty()) {
     xcommand->SetAttribute("method_name", method_name);
   }
 
+  TiXmlElement *xobject = _session->p3dobj_to_xml(this);
+  xobject->SetValue("object");
+  xcommand->LinkEndChild(xobject);
+
   for (int i = 0; i < num_params; ++i) {
     TiXmlElement *xparams = _session->p3dobj_to_xml(params[i]);
     xcommand->LinkEndChild(xparams);

direct/src/plugin/p3dPythonRun.cxx

@@ -32,6 +32,8 @@ P3DPythonRun(int argc, char *argv[]) {
   INIT_LOCK(_commands_lock);
   INIT_THREAD(_read_thread);
 
+  _next_sent_id = 0;
+
   _program_name = argv[0];
   _py_argc = 1;
   _py_argv = (char **)malloc(2 * sizeof(char *));
@@ -321,9 +323,10 @@ handle_pyobj_command(TiXmlElement *xcommand, bool needs_response,
     } else if (strcmp(op, "call") == 0) {
       // Call the named method on the indicated object, or the object
       // itself if method_name isn't given.
-      int object_id;
+      TiXmlElement *xobject = xcommand->FirstChildElement("object");
-      if (xcommand->QueryIntAttribute("object_id", &object_id) == TIXML_SUCCESS) {
+      if (xobject != NULL) {
-        PyObject *obj = (PyObject *)(void *)object_id;
+        PyObject *obj = xml_to_pyobj(xobject);
+
         const char *method_name = xcommand->Attribute("method_name");
 
         // Build up a list of params.
@@ -433,6 +436,8 @@ handle_pyobj_command(TiXmlElement *xcommand, bool needs_response,
           xresponse->LinkEndChild(pyobj_to_xml(result));
           Py_DECREF(result);
         }
+
+        Py_DECREF(obj);
       }
     }
   }
@@ -989,12 +994,25 @@ pyobj_to_xml(PyObject *value) {
     // This is more expensive for the caller to deal with--it requires
     // a back-and-forth across the XML pipe--but it's much more
     // general.
-    // TODO: pass pointers better.
-    xvalue->SetAttribute("type", "python");
-    xvalue->SetAttribute("object_id", (int)(intptr_t)value);
 
-    // TODO: fix this hack, properly manage these reference counts.
+    int object_id = _next_sent_id;
+    ++_next_sent_id;
+    bool inserted = _sent_objects.insert(SentObjects::value_type(object_id, value)).second;
+    while (!inserted) {
+      // Hmm, we must have cycled around the entire int space?  Either
+      // that, or there's a logic bug somewhere.  Assume the former,
+      // and keep looking for an empty slot.
+      object_id = _next_sent_id;
+      ++_next_sent_id;
+      inserted = _sent_objects.insert(SentObjects::value_type(object_id, value)).second;
+    }
+
+    // Now that it's stored in the map, increment its reference count.
+    // TODO: implement removing things from this map.
     Py_INCREF(value);
+
+    xvalue->SetAttribute("type", "python");
+    xvalue->SetAttribute("object_id", object_id);
   }
 
   return xvalue;
@@ -1053,12 +1071,23 @@ xml_to_pyobj(TiXmlElement *xvalue) {
   } else if (strcmp(type, "python") == 0) {
     int object_id;
     if (xvalue->QueryIntAttribute("object_id", &object_id) == TIXML_SUCCESS) {
-      return (PyObject *)(void *)object_id;
+      SentObjects::iterator si = _sent_objects.find(object_id);
+      PyObject *result;
+      if (si == _sent_objects.end()) {
+        // Hmm, the parent process gave us a bogus object ID.
+        result = _undefined;
+      } else {
+        result = (*si).second;
+      }
+      Py_INCREF(result);
+      return result;
     }
   }
 
   // Something went wrong in decoding.
-  return Py_BuildValue("");
+  PyObject *result = _undefined;
+  Py_INCREF(result);
+  return result;
 }
 
 ////////////////////////////////////////////////////////////////////

direct/src/plugin/p3dPythonRun.h

@@ -117,6 +117,14 @@ private:
 
   PT(GenericAsyncTask) _check_comm_task;
 
+  // This map keeps track of the PyObject pointers we have delivered
+  // to the parent process.  We have to hold the reference count on
+  // each of these until the parent process tells us it's safe to
+  // release them.
+  typedef pmap<int, PyObject *> SentObjects;
+  SentObjects _sent_objects;
+  int _next_sent_id;
+
   // The remaining members are manipulated by the read thread.
   typedef pdeque<TiXmlDocument *> Commands;
   Commands _commands;

direct/src/plugin/p3dSession.cxx

@@ -41,6 +41,8 @@ P3DSession(P3DInstance *inst) {
   _session_key = inst->get_session_key();
   _python_version = inst->get_python_version();
 
+  _next_sent_id = 0;
+
   _p3dpython_running = false;
   _next_response_id = 0;
   _response = NULL;
@@ -359,7 +361,13 @@ xml_to_p3dobj(const TiXmlElement *xvalue) {
   } else if (strcmp(type, "browser") == 0) {
     int object_id;
     if (xvalue->QueryIntAttribute("object_id", &object_id) == TIXML_SUCCESS) {
-      P3D_object *obj = (P3D_object *)object_id;
+      SentObjects::iterator si = _sent_objects.find(object_id);
+      if (si == _sent_objects.end()) {
+        // Hmm, the child process gave us a bogus object ID.
+        return new P3DUndefinedObject;
+      }
+
+      P3D_object *obj = (*si).second;
       return P3D_OBJECT_COPY(obj);
     }
 
@@ -371,7 +379,7 @@ xml_to_p3dobj(const TiXmlElement *xvalue) {
   }
 
   // Something went wrong in decoding.
-  return new P3DNoneObject;
+  return new P3DUndefinedObject;
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -434,13 +442,25 @@ p3dobj_to_xml(const P3D_object *obj) {
       // Otherwise, it must a host-provided object, which means we
       // should pass a reference down to this particular object, so
       // the Python process knows to call back up to here to query it.
-      // TODO: pass pointers better.  Fix this hideous leak.
+
       P3D_object *dup = P3D_OBJECT_COPY(obj);
-      int object_id = (int)(intptr_t)dup;
+
+      int object_id = _next_sent_id;
+      ++_next_sent_id;
+      bool inserted = _sent_objects.insert(SentObjects::value_type(object_id, dup)).second;
+      while (!inserted) {
+        // Hmm, we must have cycled around the entire int space?  Either
+        // that, or there's a logic bug somewhere.  Assume the former,
+        // and keep looking for an empty slot.
+        object_id = _next_sent_id;
+        ++_next_sent_id;
+        inserted = _sent_objects.insert(SentObjects::value_type(object_id, dup)).second;
+      }
+
+      // TODO: implement removing things from this map.
+
       xvalue->SetAttribute("type", "browser");
       xvalue->SetAttribute("object_id", object_id);
-
-      // TODO: manage this reference count somehow.
     }
     break;
   }

direct/src/plugin/p3dSession.h

@@ -107,6 +107,13 @@ private:
   typedef vector<TiXmlDocument *> Commands;
   Commands _commands;
 
+  // This map keeps track of the P3D_object pointers we have delivered
+  // to the child process.  We have to keep each of these until the
+  // child process tells us it's safe to delete them.
+  typedef map<int, P3D_object *> SentObjects;
+  SentObjects _sent_objects;
+  int _next_sent_id;
+
   P3DPackage *_panda3d;
   PackageCallback *_panda3d_callback;