agraef / pd-lua

Lua bindings for Pd, updated for Lua 5.3+
https://agraef.github.io/pd-lua/
GNU General Public License v2.0
47 stars 10 forks source link

New dsp perform routine segfaults if there is no signal inlet #44

Closed agraef closed 6 months ago

agraef commented 6 months ago

I noticed this while playing around with the lua_sig example. The attached variation of that example simply segfaults if you turn on dsp in Pd, probably because of the missing in1 table argument? Here's the output from the terminal when running pd lua_sig-test.pd and turning on dsp:

PANIC: unprotected error in call to Lua API (attempt to call a nil value)
Pd: signal 6

Looking at the C code for pdlua_perform(), it seems that only the sample blocks from the signal inlets are passed to the perform method on the Lua side, not the outputs. So if there is no signal inlet, the Lua perform routine has no way of knowing how many samples to produce, and how many blocks of samples.

I think that this needs to be reworked, so that the Lua perform routine gets both the input and output data (as two tables of tables maybe). At the very least the routine needs to know exactly how many outputs there are and how large the block size is. And it should never segfault, no matter how badly garbled the Lua code is. :stuck_out_tongue_winking_eye:

@timothyschoen Can you please have a look?

lua_sig.zip

agraef commented 6 months ago

Thinking about it some more, of course the Lua object knows about the numbers of inputs and outputs, since that information is in the self.inlets and self.outlet members, silly me.

So it should probably be enough to pass in tables for both the input and the output buffers. That should be easy enough to fix, working on it.

agraef commented 6 months ago

Thinking about it some more, of course the Lua object knows about the numbers of inputs and outputs, since that information is in the self.inlets and self.outlet members, silly me.

Well, I also missed that the block size is being passed to the dsp method on the Lua side.

So it should probably be enough to pass in tables for both the input and the output buffers.

That seems to do the trick. Not sure whether it's the right thing to do, though. A Lua signal object without a signal inlet can also just create a table of the right size and then return that.

Also, I'm still worried about that PANIC message with the subsequent segfault (or rather signal 6 = abort). That seems to happen inside the Lua interpreter whenever the Lua perform method tries to access a non-existing argument. This will obviously trigger an exception in the Lua interpreter, but I thought that pcall is supposed to catch and handle this?

timothyschoen commented 6 months ago

Also, I'm still worried about that PANIC message with the subsequent segfault (or rather signal 6 = abort). That seems to happen inside the Lua interpreter whenever the Lua perform method tries to access a non-existing argument. This will obviously trigger an exception in the Lua interpreter, but I thought that pcall is supposed to catch and handle this?

Figured out the problem: when the call to perform fails, we catch that error, but we don't return early. So it will still try to unpack the output of the function, even though the function call failed. Should be fixed with 46f9eae. Now it will just tell you without crashing:

pdlua: error in perform:
[string "lua_sig"]:34: attempt to get length of a nil value (local 'in1')

We can also considering filling the output buffers with 0 when the function call fails, just to be sure.

agraef commented 6 months ago

Yeah, just figured that out as well. ;-) https://github.com/agraef/pd-lua/commit/46f9eaee33ba77b19fa8e5207381e47d4f4ba203 should be fine as is, no need to fill the buffers.

agraef commented 6 months ago

Fixed in #43.