Cycling74 / min-api

High-level C++-language application programming interface for Max
MIT License
58 stars 23 forks source link

crashes in buffer_reference and buffer_lock #173

Open x37v opened 4 years ago

x37v commented 4 years ago

checkout and build https://github.com/Cycling74/min-devkit/tree/xnor/buffer-bugs

Debug build makes sense

mkdir build && cd build && cmake -DCMAKE_BUILD_TYPE=Debug .. && make -j8

bug 1

  1. start max
  2. create an object min.buffer.cons~
  3. delete that object .. crash

bug 2

  1. start max
  2. create buffer object buffer~ test1 1000
  3. create object min.buffer.lck~ test1
  4. start dsp
  5. delete buffer~ object.. crash
x37v commented 4 years ago

It looks like both of these bugs are related to how buffer ref hijacks the owner object's 'notify' method.

In the first one, the buffer object holds the 'notify' message, so when it is deallocated it deletes that message object, but there are still callbacks that are dispatched to that message, so its an access after delete issue.

In the second one, each buffer allocation creates its own 'notify' message, but it looks like only the last one actually gets used, if you create a buffer~ test2 1000 and delete that once dsp is started, you don't get the crash. So it looks like the 'notify' messages buffers get for binding/unbinding get to the buffer reference for test2 but not the one for test1.

Maybe buffer references in min should actually be objects that can manage these bind/unbind notify messages themselves and not hijack the notify of the owner?

x37v commented 4 years ago

See: https://github.com/Cycling74/min-api/blob/f34cedef103ba5f30a1a02cfa0a995508b710036/include/c74_min_buffer.h#L94:L108

buffer_reference registers the 'notify' method for its owning object. If its owner has multiple buffer_references, only the final reference gets this logic called. I think this behavior exists elsewhere in min as well.

@tap has a branch that makes this message registration for buffer_reference optional.. and that solves bug 1 above, but then the references aren't aware if they're bound or unbound so you get crashes deallocating buffers in max while your owner exists.