Hi all
I noticed that [packOSC] does not support reentrancy (check attached patch).
Many object classes _do_ support reentrancy. At the same time it seems common in other environments not to support reentrancy. I wonder if this is considered bug. If so, I'll file a bug report.
Roman
What do you expect the patch to do, hang Pd? For me it prints this in the Pd 0.46.7 console: packOSC: This packet is not a bundle, so you can't write another address packOSC: Problem writing address. packOSC: According to the type tag I didn't expect any more arguments. packOSC: usage error, write-msg failed. print: /trigger/osc/message now It's possible to open a bundle (with a '[' message) and add multiple messages, but the OSC message is output only when the bundle is closed with the ']' message. In your patch, if you put a [delay] after the bang outlet it will start an infinite loop at the delay interval. I'm not sure why the delay is necessary though, as [packOSC] should have finished processing the previous message by the time the next one arrives, even if it's a zero logical time delay.
Martin
On Thu, Nov 5, 2015 at 7:54 AM, Roman Haefeli reduzent@gmail.com wrote:
Hi all
I noticed that [packOSC] does not support reentrancy (check attached patch).
Many object classes _do_ support reentrancy. At the same time it seems common in other environments not to support reentrancy. I wonder if this is considered bug. If so, I'll file a bug report.
Roman
Pd-list@lists.iem.at mailing list UNSUBSCRIBE and account-management -> http://lists.puredata.info/listinfo/pd-list
On Thu, 2015-11-05 at 11:11 -0500, Martin Peach wrote:
What do you expect the patch to do, hang Pd?
You're right. The patch would cause a stack overflow. I sent another one.
For me it prints this in the Pd 0.46.7 console: packOSC: This packet is not a bundle, so you can't write another address packOSC: Problem writing address. packOSC: According to the type tag I didn't expect any more arguments. packOSC: usage error, write-msg failed. print: /trigger/osc/message now
It's possible to open a bundle (with a '[' message) and add multiple messages, but the OSC message is output only when the bundle is closed with the ']' message.
Yeah, but I don't want to start a bundle. I think [packOSC] should be done processing before sending the message to the outlet, but maybe there is a reason it is in a special state when sending something to the outlet?
In your patch, if you put a [delay] after the bang outlet it will start an infinite loop at the delay interval. I'm not sure why the delay is necessary though, as [packOSC] should have finished processing the previous message by the time the next one arrives, even if it's a zero logical time delay.
So, you think this is a bug?
Roman
On Thu, 2015-11-05 at 11:11 -0500, Martin Peach wrote:
In your patch, if you put a [delay] after the bang outlet it will start an infinite loop at the delay interval. I'm not sure why the delay is necessary though, as [packOSC] should have finished processing the previous message by the time the next one arrives, even if it's a zero logical time delay.
Attached patch illustrates that you can simply buffer the output of [packOSC] with [list append]. This fixes the reentrancy problem _without_ affecting the execution order. A zero logical time delay breaks the depth-first rule.
Roman
As I see it, the only way that [packOSC] can receive a message before it has completed the previous message is if its own output triggers another message to its input. It may be possible to detect that special case and either let it happen or not, but it's tricky because the buffer for the list is deallocated as soon as the list is output, and the new message arrives while that is taking place. Possibly using a fixed buffer would work better, and also speed up the code. The function then might just exit before it is called again, avoiding the error.
Martin
On Thu, Nov 5, 2015 at 1:21 PM, Roman Haefeli reduzent@gmail.com wrote:
On Thu, 2015-11-05 at 11:11 -0500, Martin Peach wrote:
In your patch, if you put a [delay] after the bang outlet it will start an infinite loop at the delay interval. I'm not sure why the delay is necessary though, as [packOSC] should have finished processing the previous message by the time the next one arrives, even if it's a zero logical time delay.
Attached patch illustrates that you can simply buffer the output of [packOSC] with [list append]. This fixes the reentrancy problem _without_ affecting the execution order. A zero logical time delay breaks the depth-first rule.
Roman
Pd-list@lists.iem.at mailing list UNSUBSCRIBE and account-management -> http://lists.puredata.info/listinfo/pd-list
On Thu, 2015-11-05 at 16:23 -0500, Martin Peach wrote:
As I see it, the only way that [packOSC] can receive a message before it has completed the previous message is if its own output triggers another message to its input. It may be possible to detect that special case and either let it happen or not, but it's tricky because the buffer for the list is deallocated as soon as the list is output, and the new message arrives while that is taking place. Possibly using a fixed buffer would work better, and also speed up the code. The function then might just exit before it is called again, avoiding the error.
If you're trying a fix, I'm happy to test it (Sorry for not being helpful, my C skills... )
Roman
So I updated packOSC in svn to detect reentrancy and post a message. It seems too complicated to allow it. Here's a patch that does what I believe you want using the preferred method of bundles.
Martin
On Fri, Nov 6, 2015 at 4:53 AM, Roman Haefeli reduzent@gmail.com wrote:
On Thu, 2015-11-05 at 16:23 -0500, Martin Peach wrote:
As I see it, the only way that [packOSC] can receive a message before it has completed the previous message is if its own output triggers another message to its input. It may be possible to detect that special case and either let it happen or not, but it's tricky because the buffer for the list is deallocated as soon as the list is output, and the new message arrives while that is taking place. Possibly using a fixed buffer would work better, and also speed up the code. The function then might just exit before it is called again, avoiding the error.
If you're trying a fix, I'm happy to test it (Sorry for not being helpful, my C skills... )
Roman
Pd-list@lists.iem.at mailing list UNSUBSCRIBE and account-management -> http://lists.puredata.info/listinfo/pd-list
Hey, thanks for addressing the issue.
On Mon, 2015-11-16 at 15:25 -0500, Martin Peach wrote:
So I updated packOSC in svn to detect reentrancy and post a message.
The message says: error: packOSC: Use bundle to send multiple messages
I find this message a bit misleading as I'm not at all trying to create a bundle. I'm simply creating an OSC packet that in returns triggers another OSC packet to be generated. I expect [packOSC] to treat them absolutely separately.
It seems too complicated to allow it.
I see. It's not complicated at all on a patch level to make [packOSC] allow reentrancy. See attached patch. I can live easily without it being fixed in [packOSC].
And this is the _expected_ output from my previous patch (and it actually _is_ the output of the currently attached patch with the work-around):
FIRST: /i/am/reentrant 64 SECOND: now
Here's a patch that does what I believe you want using the preferred method of bundles.
That's not quite what I want. I don't mean to create a bundle. For certain reasons it's hard to avoid reentrancy in the framework I'm working on, but buffering the output of [packOSC] as shown in the attached patch works.
Roman
On 2015-11-17 10:15, Roman Haefeli wrote:
It seems too complicated to allow it.
I see. It's not complicated at all on a patch level to make [packOSC] allow reentrancy. See attached patch. I can live easily without it being fixed in [packOSC].
i forgot whether i already mentioned this, but [oscformat] does not have the reentrancy problem (though there may be plenty of other reasons to stick with [packOSC])
fgamsdr IOhannes
On Tue, 2015-11-17 at 10:58 +0100, IOhannes m zmoelnig wrote:
On 2015-11-17 10:15, Roman Haefeli wrote:
It seems too complicated to allow it.
I see. It's not complicated at all on a patch level to make [packOSC] allow reentrancy. See attached patch. I can live easily without it being fixed in [packOSC].
i forgot whether i already mentioned this, but [oscformat] does not have the reentrancy problem (though there may be plenty of other reasons to stick with [packOSC])
Thanks for the reminder. I think you already mentioned it. The main reason I stick with [packOSC] (at least for now) is that the whole framework is based on a /this/is/an/address syntax, while [oscformat] uses 'set this is an address'.
The only other major difference I can see between the two is that [oscformat] does not support bundles (I don't use them).
Roman
On Tue, Nov 17, 2015 at 4:15 AM, Roman Haefeli reduzent@gmail.com wrote:
Hey, thanks for addressing the issue.
On Mon, 2015-11-16 at 15:25 -0500, Martin Peach wrote:
So I updated packOSC in svn to detect reentrancy and post a message.
The message says: error: packOSC: Use bundle to send multiple messages
I find this message a bit misleading as I'm not at all trying to create a bundle. I'm simply creating an OSC packet that in returns triggers another OSC packet to be generated. I expect [packOSC] to treat them absolutely separately.
What should it say then? "packOSC is being asked to process another message before it is finished with the current one"?
It seems too complicated to allow it.
...but I'll keep thinking about it. There may be a way.
I see. It's not complicated at all on a patch level to make [packOSC] allow reentrancy. See attached patch. I can live easily without it being fixed in [packOSC].
And this is the _expected_ output from my previous patch (and it actually _is_ the output of the currently attached patch with the work-around):
FIRST: /i/am/reentrant 64 SECOND: now
Here's another patch that does the same thing by using two [packOSC]s.
Martin
On Wed, Nov 18, 2015 at 11:56 AM, Martin Peach chakekatzil@gmail.com wrote:
On Tue, Nov 17, 2015 at 4:15 AM, Roman Haefeli reduzent@gmail.com wrote:
Hey, thanks for addressing the issue.
On Mon, 2015-11-16 at 15:25 -0500, Martin Peach wrote:
It seems too complicated to allow it.
...but I'll keep thinking about it. There may be a way.
So I think I got it, it's in the latest svn revision. I hope it didn't break something else, but [packOSC] can now send messages triggered by its own output.
Martin
On 2015-11-18 21:31, Martin Peach wrote:
So I think I got it, it's in the latest svn revision. I hope it didn't break something else, but [packOSC] can now send messages triggered by its own output.
i had a quick glance at the code (didn't even try it out), but it seems you are mainly pushing the problem a little bit "back" rather than solving it. that is: the output of [packOSC] can trigger a *single* message that is feed back (via the stack) to it's own input. it is not possible to have that 2nd message trigger a feedback again.
i don't think it is valid to just dismiss the problem, simply because you (or I, for that matter) don't have an actual use case at hand (roman has come up with one that requires first-level recursion, though probably not deep recursion)
anyhow, i think the problem shouldn't be too hard to solve. afaics, the main issue is, that (for performance reasons) you use an intermediate "OSC buffer" that is allocated on the heap (and which will get overwritten in recursion). the solution is obviously to use a stack allocated buffer.
to keep the performance of a heap allocated buffer, i would suggest to:
this will make sure that a simple use of [packOSC] will be as fast as ever, while at the same time allowing "infinite" recursion (bound by the environment, not by [packOSC]) - albeit users of recursion will have to live with a small performance penalty.
fgmsdr IOhannes
¹ see my other mail on [unpackOSC]
On 2015-11-19 09:49, IOhannes m zmoelnig wrote:
i had a quick glance at the code (didn't even try it out),
now that i did try it out, it seems that something is going wrong, since when i do deeper recursion, i either get weirdo printout or segfaults (which makes me think of memory corruption).
weirdo printout:
packOSC: According to the type tag (�) I expected more arguments. error: packOSC: Problem writing address. packOSC: According to the type tag I expected an argument of a
different type.
- Expected i, string now ���
error: packOSC: usage error, packOSC_writemessage failed.
looking at the code i expected something along the lines: "reentry count 2 exceeds limit" instead...
dfmasd IOhannes
On 2015-11-17 10:15, Roman Haefeli wrote:
I see. It's not complicated at all on a patch level to make [packOSC] allow reentrancy. See attached patch. I can live easily without it being fixed in [packOSC].
your solution unfortunately shows that [unpackOSC] is broken as well.
since messages seems to be buffered internally in some heap-allocated memory, [unpackOSC] egalize all recursive messages to the last received. the problem appears when using
see attached patch.
fgamsdr IOhannes
Oops.. the test patch is bogus. If [packOSC] wouldn't throw away the second message, the patch would trigger a stack overflow. Attached is a patch that wouldn't trigger any problems if [packOSC] would support reentrancy.
Roman
On Thu, 2015-11-05 at 13:54 +0100, Roman Haefeli wrote:
Hi all
I noticed that [packOSC] does not support reentrancy (check attached patch).
Many object classes _do_ support reentrancy. At the same time it seems common in other environments not to support reentrancy. I wonder if this is considered bug. If so, I'll file a bug report.
Roman