[Bug 769] New: Logic errors in Timer

Submitted by snrkiwi on Fri, 2010-05-28 18:56

RTT-dev

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

Summary: Logic errors in Timer
Product: RTT
Version: rtt-trunk
Platform: All
OS/Version: GNU/Linux
Status: NEW
Severity: normal
Priority: P3
Component: Operating System Abstraction - Portability
AssignedTo: orocos-dev [..] ...
ReportedBy: kiwi [dot] net [..] ...
CC: orocos-dev [..] ...
Estimated Hours: 0.0

I think the RTT::Timer::loop() code has a couple of logic errors. I've attached
a patch that shows step by step what happens with the time_test code.
Basically, 1) the return value of msem.waitUntil() is bool and not int, and 2)
the logic of handling timeout's currently makes you one iteration late when
announcing the timeout. See attached.

[Bug 769] Logic errors in Timer

Submitted by peter on Tue, 2010-12-21 10:24.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

--- Comment #6 from Peter Soetens <peter [..] ...> 2010-12-21 11:21:10 ---
(In reply to comment #5)
> Thanks for the digging. I missed this one for the 1.12 release. Since the
> release-virtual-clocks branch makes some TimeService functions virtual, I can
> not merge this branch for 1.12.1 without breaking ABI.
>
> I'll merge this on 1.x trunk and cherry-pick the Timer fixes for the 1.12
> branch. The full set of patches is then for 1.14.

Oops. Forgot this is git-svn. I can't merge, only fast-forward a branch. Do you
want to rebase this on the latest 1.0-svn-patches or shall I do so ?

Peter

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Tue, 2010-12-21 19:48.

On 21/12/2010, at 23:21 , Peter Soetens wrote:

> https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769
>
>
>
>
>
> --- Comment #6 from Peter Soetens <peter [..] ...> 2010-12-21 11:21:10 ---
> (In reply to comment #5)
>> Thanks for the digging. I missed this one for the 1.12 release. Since the
>> release-virtual-clocks branch makes some TimeService functions virtual, I can
>> not merge this branch for 1.12.1 without breaking ABI.
>>
>> I'll merge this on 1.x trunk and cherry-pick the Timer fixes for the 1.12
>> branch. The full set of patches is then for 1.14.
>
> Oops. Forgot this is git-svn. I can't merge, only fast-forward a branch. Do you
> want to rebase this on the latest 1.0-svn-patches or shall I do so ?
>
> Peter

If you want this in the next 2 weeks, you'll have to do yourself unfortunately. I'm on holiday .... can do after I return if you want.
S

[Bug 769] Logic errors in Timer

Submitted by peter on Tue, 2010-12-21 10:16.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

Peter Soetens <peter [..] ...> changed:

--- Comment #5 from Peter Soetens <peter [..] ...> 2010-12-21 11:15:07 ---
Thanks for the digging. I missed this one for the 1.12 release. Since the
release-virtual-clocks branch makes some TimeService functions virtual, I can
not merge this branch for 1.12.1 without breaking ABI.

I'll merge this on 1.x trunk and cherry-pick the Timer fixes for the 1.12
branch. The full set of patches is then for 1.14.

Peter

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Wed, 2010-06-16 20:48.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

--- Comment #4 from S Roderick <kiwi [dot] net [..] ...> 2010-06-16 22:47:16 ---
There were also several logic errors in the macosx fosi layer to do with
semaphore timed waits. In fact, it's a surprise it ever worked - it only did
beacuse it would busy spin the CPU in the background (it was a copy/paste erorr
in os/macosx/fosi.h, coupled with thinking a value was unsigned).

I'm not sending these in as patches. Check out the "release-virtual-clocks"
branch of my github.

git [..] ...:snrkiwi/orocos-rtt.git

I have also added a scaled clock interactive test.

Tested and demonstrated on Snow Leopard and Ubuntu Lynx.

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Thu, 2010-06-17 00:40.

On Jun 16, 2010, at 16:47 , S Roderick wrote:

> https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769
>
>
>
>
>
> --- Comment #4 from S Roderick <kiwi [dot] net [..] ...> 2010-06-16 22:47:16 ---
> There were also several logic errors in the macosx fosi layer to do with
> semaphore timed waits. In fact, it's a surprise it ever worked - it only did
> beacuse it would busy spin the CPU in the background (it was a copy/paste erorr
> in os/macosx/fosi.h, coupled with thinking a value was unsigned).
>
> I'm not sending these in as patches. Check out the "release-virtual-clocks"
> branch of my github.
>
> git [..] ...:snrkiwi/orocos-rtt.git
>
> I have also added a scaled clock interactive test.
>
> Tested and demonstrated on Snow Leopard and Ubuntu Lynx.

Use the much cleaner "next-virtual-clocks" branch instead of "release-virtual-clocks".
S

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Fri, 2010-05-28 20:24.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

--- Comment #3 from S Roderick <kiwi [dot] net [..] ...> 2010-05-28 22:19:48 ---
Created an attachment (id=604)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=604)
Patch to clean up both noted errors

This also avoids the year 2038 problem a little better (though it is still
there in the underlying O/S). I think the logic is also cleaner. Some of these
changes will be needed for virtual clocks/time-services ...

Tested on Lynx and Snow Leopard.

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Fri, 2010-05-28 19:16.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

--- Comment #2 from S Roderick <kiwi [dot] net [..] ...> 2010-05-28 21:13:04 ---
Created an attachment (id=603)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=603)
Log from patched system

Patching with

                ret = msem.waitUntil( wake_up_time ) ? 0 : -1; // case of no
timers or running timers
                printf("waitUntil returned %d, %d\n", errno, ret);
fflush(stdout);

and you can see that it times out on the same loop correctly.

NB I still think there are logic errors in here, but this is a step ...

[Bug 769] Logic errors in Timer

Submitted by snrkiwi on Fri, 2010-05-28 19:12.

https://www.fmtc.be/bugzilla/orocos/show_bug.cgi?id=769

--- Comment #1 from S Roderick <kiwi [dot] net [..] ...> 2010-05-28 21:09:22 ---
Created an attachment (id=602)
--> (https://www.fmtc.be/bugzilla/orocos/attachment.cgi?id=602)
Instrument Timer::loop() and time-test

Note the additional "go" between waitUntil and the timeout. For Ubuntu,
errno=110 is the result of a timed out semaphore, which should make re=-1
instead of 0.