SegFault when using AsyncFuture

Scripting Issues
1

Some context: I have a task that is responsible for generating procedural data on the GPU and store it in a texture. As I don’t want to generate dozen of textures per frame, I created a task that will fetch the requests from a queue, and notify the waiting task once the generation is done. To perform this notification I’m trying to use a future. The requesting task will await on the future and the generator task will use set_result to notify that task that the texture is ready (and send back the texture).

However while testing this, I encountered several weird stuff, and often segfault; Here is a code that tries to reproduce the problem :

from panda3d.core import AsyncFuture, Texture

queue = []

def done_task(task):
    if len(queue) > 0:
        print("POP")
        (future, texture) = queue.pop(0)
        future.set_result(texture)
    return task.cont

def generate():
    future = AsyncFuture()
    queue.append((future, Texture()))
    return future

async def main_task():
    result = await generate()
    print("DONE")
    print(result)
    t = result.get_result()
    print(t)

def add_task():
    taskMgr.add(done_task, "done", sort=-10000)
    taskMgr.add(main_task(), 'ex_test')

import direct.directbase.DirectStart

add_task()
base.run()

This usually cause the following error :

|0   core.cpython-37m-darwin.so    |0x000000010711b179 get_done_result(AsyncFuture const*) + 633|
|---|---|
|1   core.cpython-37m-darwin.so    |0x000000010711ad1b gen_next(_object*) + 59|
|2   org.python.python             |0x0000000106944433 _PyEval_EvalFrameDefault + 9907|

On a side note, it’s not possible to send arbitrary Python data using set_result, it seems it must be an instance of a Panda3D object.

2

Hmm, I get this when I run it:

:task(error): PythonTask ex_test exception was never retrieved:
Traceback (most recent call last):
  File "/tmp/test.py", line 21, in main_task
    t = result.get_result()
AttributeError: 'panda3d.core.Texture' object has no attribute 'get_result'

…which makes sense, because await returns the result already, not the future itself.

Is there something different about your build? Are you compiling with optimizations?

3

I have the problem also with the official wheels, optimised and non optimised. But this is on mac only, I haven’t tested on other platforms yet.

Indeed, I was just checking if I got the future instead of the result, but the code crashes before even that actually.

Note that one time I got the result without a crash, and instead of displaying the texture object, I got a handle on a TypedWritable object instead.

<panda3d.core.TypedWritable object at 0x1100047e0>

4

Instrumenting the code, the crash is in AsyncFuture_ext.cpp :

static PyObject *get_done_result(const AsyncFuture *future) {
      ...
      TypeHandle type = ptr->get_type();

The pointer value is still the same as the one set in set_result(), so something in the texture object has been corrupted.

On a side note, I don’t know if that’s expected or not, but ref_ptr is a nullptr. It seems that the code has used set_result(TypedObject *result) I would have expected set_result(TypedReferenceCount *result) as I believe Texture inherit from ReferenceCount.

Edit: calling get_type() in set_result() works and correctly returns ‘Texture’ so the object is either corrupted or garbage collected before reaching get_done_result()

5

You can try putting an abort() in the texture destructor and get a stack trace to see when it gets destroyed.

However, ref_ptr is supposed to be set for correct reference management, so I wonder if this is an issue with selecting the correct overload. I suppose you are calling set_result() from Python, so the overload resolution is done by interrogate—maybe interrogate has a bug in its overload resolution? Did you compile Panda yourself, if so, can I see the libp3event_igate.cxx?

Hiding the set_result() overload that takes a TypedObject from interrogate might do it.

6

Indeed, if I keep a reference to the generated texture, the code above works fine, so it’s definitively a reference count problem.

If Ì comment out the section related to TypedObject in libp3event_igate.cxx it also solves the problem. I guess hiding from interrogate that overload would do the trick.

Here is the full set_result wrapper generated by interrogate :

static PyObject *Dtool_AsyncFuture_set_result_37(PyObject *self, PyObject *arg) {
  AsyncFuture *local_this = nullptr;
  if (!Dtool_Call_ExtractThisPointer_NonConst(self, Dtool_AsyncFuture, (void **)&local_this, "AsyncFuture.set_result")) {
    return nullptr;
  }
  {
    // -2 inline void AsyncFuture::set_result(EventParameter const &result)
    EventParameter const *arg_this = nullptr;
    DtoolInstance_GetPointer(arg, arg_this, *Dtool_Ptr_EventParameter);
    if (arg_this != nullptr) {
      ((*local_this).set_result)(*arg_this);
      return Dtool_Return_None();
    }
  }

  {
    // -2 inline void AsyncFuture::set_result(TypedObject *result)
    TypedObject *arg_this = (TypedObject *)DTOOL_Call_GetPointerThisClass(arg, Dtool_Ptr_TypedObject, 1, "AsyncFuture.set_result", false, false);
    if (arg_this != nullptr) {
      ((*local_this).set_result)(arg_this);
      return Dtool_Return_None();
    }
  }

  {
    // -2 inline void AsyncFuture::set_result(TypedReferenceCount *result)
    TypedReferenceCount *arg_this = (TypedReferenceCount *)DTOOL_Call_GetPointerThisClass(arg, Dtool_Ptr_TypedReferenceCount, 1, "AsyncFuture.set_result", false, false);
    if (arg_this != nullptr) {
      ((*local_this).set_result)(arg_this);
      return Dtool_Return_None();
    }
  }

  {
    // -2 inline void AsyncFuture::set_result(TypedWritableReferenceCount *result)
    TypedWritableReferenceCount *arg_this = (TypedWritableReferenceCount *)DTOOL_Call_GetPointerThisClass(arg, Dtool_Ptr_TypedWritableReferenceCount, 1, "AsyncFuture.set_result", false, false);
    if (arg_this != nullptr) {
      ((*local_this).set_result)(arg_this);
      return Dtool_Return_None();
    }
  }

  {
    // -2 inline void AsyncFuture::set_result(std::nullptr_t )
    if (arg == Py_None) {
      ((*local_this).set_result)(nullptr);
      return Dtool_Return_None();
    }
  }

  {
    // -2 inline void AsyncFuture::set_result(EventParameter const &result)
    EventParameter arg_local;
    EventParameter const *arg_this = Dtool_Coerce_EventParameter(arg, arg_local);
    if ((arg_this != nullptr)) {
      ((*local_this).set_result)(*arg_this);
      return Dtool_Return_None();
    }
  }

  // No coercion possible: inline void AsyncFuture::set_result(TypedObject *result)
  // No coercion possible: inline void AsyncFuture::set_result(TypedReferenceCount *result)
  // No coercion possible: inline void AsyncFuture::set_result(TypedWritableReferenceCount *result)
  // No coercion possible: inline void AsyncFuture::set_result(std::nullptr_t )
  if (!_PyErr_OCCURRED()) {
    return Dtool_Raise_BadArgumentsError(
      "set_result(const AsyncFuture self, const EventParameter result)\n"
      "set_result(const AsyncFuture self, TypedObject result)\n"
      "set_result(const AsyncFuture self, TypedReferenceCount result)\n"
      "set_result(const AsyncFuture self, TypedWritableReferenceCount result)\n"
      "set_result(const AsyncFuture self, NoneType param0)\n");
  }
  return nullptr;
}
7

Some code in interrogate to sort overloads (so that more derived types would come before less derived types) wasn’t working right. I checked in a fix:

Thanks for your help testing this and reporting issues!

1 Like
8

Side question, that perhaps warrants its own discussion thread, set_result() requires a “pure” Panda3D object, is it possible to add support for plain Python object too or is there some limitations that prevent this ? (Or is it a way to piggyback the Python object using a Panda object ?)

9

Yeah, we can support something like that. We just need to create an object inheriting ParamValueBase that wraps a PyObject pointer.

10

All right, this should do it:

1 Like
11

Wow, thanks a lot ! (I was about to say that in the meantime one could use PandaNode and set_python_tag, but I didn’t even had the time to write it that you’re back with the full implementation :slight_smile: )