tests: add tests to reproduce #10881 by miri64 · Pull Request #10908 · RIOT-OS/RIOT

miri64 · 2019-01-30T18:18:27Z

Contribution description

As the title says: This is a test to reproduce the race-condition described in #10881.

Testing procedure

Run make test on native (or for that matter try it on other platforms as well). ~~In current master, the application should run immediately into an assertion,~~ (#10891 was merged) with #10891 this is fixed (we have a TIMEOUT which we want this time).

Issues/PRs references

Reproduces #10881

miri64 · 2019-01-30T18:22:38Z

@cladmi considering #10891 (comment) can you run this on an atmega platform, please (to be absolutely sure also run just make term for a while not just make test; edit: the latter just aborts after 10 seconds without assertion)?

aabadie

Comments in the the application code wouldn't be too much I think.

tests/thread_msg_block_race/tests/01-run.py

miri64 · 2019-01-30T18:33:57Z

Comments in the the application code wouldn't be too much I think.

Added some. I hope I described the problem correctly.

miri64 · 2019-01-31T10:45:13Z

Codacy reports that the assert in the python script might be compiled out. One more reason to go for #10431 ;-).

aabadie · 2019-01-31T14:43:25Z

One more reason to go for #10431 ;-).

I disagree, using assert in tests is totally right in Python. The codacy issue is a false positive in this case. It's just that if you run your script with optimized byte code (using python3 -o), the assert will be ignored. But this optimisation is never used in this kind of tests AFAIK.

miri64 · 2019-01-31T21:40:15Z

I disagree, using assert in tests is totally right in Python. The codacy issue is a false positive in this case. It's just that if you run your script with optimized byte code (using python3 -o), the assert will be ignored. But this optimisation is never used in this kind of tests AFAIK.

Unless you are doubting the necessity of #10431 there is no disagreement. Yes, the error pointed out by Codacity is a false positive, but also using the asserts provided by TestCase is way cleaner.

miri64 · 2019-02-01T14:38:57Z

@cladmi considering #10891 (comment) can you run this on an atmega platform, please (to be absolutely sure also run just make term for a while not just make test; edit: the latter just aborts after 10 seconds without assertion)?

I did that myself now. The test ran for >1h on an arduino-atmega2560 without encountering any assertion, so with some degree of uncertainty I can say that #10891 is not an issue on atmega.

miri64 · 2019-02-01T15:31:49Z

Rebased to current master to include #10891.

miri64 · 2019-02-01T15:36:45Z

@aabadie this should also go into RC2 IMHO, so we can test the bug in the release tests.

miri64 · 2019-02-13T11:34:41Z

@kaspar030 can you have a look?

tests/thread_msg_block_race/main.c

miri64 · 2019-02-13T12:57:32Z

Addressed comments

miri64 · 2019-03-24T14:00:30Z

Ping? This would be great to have in the release!

miri64 · 2019-03-27T17:58:53Z

@kaspar030 ping?!?

cladmi

I could run the test on arduino-mega2560 frdm-k64f frdm-kw41z msba2 mulle nrf52dk nucleo-f103rb pba-d-01-kw2x sltb001a stm32f3discovery.

I did not review the code in details but the implementation seems right to me, was reviewed when fixing the code and it shows the issue when reverting #10811 locally.

The test should really be merged when the issue is there otherwise people loose interest and it is harder to verify it testing. Even maybe by disabling the test at the beginning (or other mechanism) instead of not adding a good test.

Please squash.

miri64 · 2019-03-28T14:22:38Z

Ported to PexpectTestCase to get rid of the assert vs assert() discussion once and for all (I needed to rebase for that). Can you have another look?

miri64 · 2019-03-28T14:26:11Z

(sorry, didn't noticed you ACK while you re-wrote the tests and I didn't want to open a follow-up)

cladmi · 2019-03-28T17:19:31Z

This now removes the output on failure: (with the fix reverted)

make all flash test
Building application "tests_thread_msg_block_race" for "native" with MCU "native".

"make" -C /home/harter/work/git/RIOT/boards/native
"make" -C /home/harter/work/git/RIOT/boards/native/drivers
"make" -C /home/harter/work/git/RIOT/core
"make" -C /home/harter/work/git/RIOT/cpu/native
"make" -C /home/harter/work/git/RIOT/cpu/native/periph
"make" -C /home/harter/work/git/RIOT/cpu/native/vfs
"make" -C /home/harter/work/git/RIOT/drivers
"make" -C /home/harter/work/git/RIOT/drivers/periph_common
"make" -C /home/harter/work/git/RIOT/sys
"make" -C /home/harter/work/git/RIOT/sys/luid
"make" -C /home/harter/work/git/RIOT/sys/random
"make" -C /home/harter/work/git/RIOT/sys/random/tinymt32
   text    data     bss     dec     hex filename
  31635     584   55936   88155   1585b /home/harter/work/git/RIOT/tests/thread_msg_block_race/bin/native/tests_thread_msg_block_race.elf
true
F
======================================================================
FAIL: test_message_not_written (__main__.TestMessageNotWritten)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/harter/work/git/RIOT/tests/thread_msg_block_race/tests/01-run.py", line 21, in test_message_not_written
    self.spawn.expect("Message was not written")
AssertionError: TIMEOUT not raised

----------------------------------------------------------------------
Ran 1 test in 3.370s

FAILED (failures=1)
/home/harter/work/git/RIOT/tests/thread_msg_block_race/../../Makefile.include:576: recipe for target 'test' failed
make: *** [test] Error 1

cladmi · 2019-03-28T17:26:45Z

So I think better remove the last change and go without it for the moment.

miri64 · 2019-03-28T17:41:56Z

Ok, I reverted the unittest change and squashed.

cladmi · 2019-03-28T18:03:39Z

I now again get the output.

RIOT_CI_BUILD=1 make all flash test
Building application "tests_thread_msg_block_race" for "native" with MCU "native".

   text    data     bss     dec     hex filename
  31735     584   55936   88255   158bf /home/harter/work/git/RIOT/tests/thread_msg_block_race/bin/native/tests_thread_msg_block_race.elf
true
/home/harter/work/git/RIOT/tests/thread_msg_block_race/bin/native/tests_thread_msg_block_race.elf
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: buildtest)
Test is "successful" if it runs forever without halting
on any of the assertion in this file

I will try to trigger an interrupt at random intervals. When an
interrupt is fired while ISR is disable in the thread_yield_higher()
function some platform-specific implementations used to not call
sched_run() which was the cause of the bug tested here
.

And on error (when the fix is reverted)

RIOT_CI_BUILD=1 make all flash test
Building application "tests_thread_msg_block_race" for "native" with MCU "native".

   text    data     bss     dec     hex filename
  31579     584   55936   88099   15823 /home/harter/work/git/RIOT/tests/thread_msg_block_race/bin/native/tests_thread_msg_block_race.elf
true 
/home/harter/work/git/RIOT/tests/thread_msg_block_race/bin/native/tests_thread_msg_block_race.elf  
RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: buildtest)
Test is "successful" if it runs forever without halting
on any of the assertion in this file

I will try to trigger an interrupt at random intervals. When an
interrupt is fired while ISR is disable in the thread_yield_higher()
function some platform-specific implementations used to not call
sched_run() which was the cause of the bug tested here
Message was not written
...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................
Traceback (most recent call last):
  File "/home/harter/work/git/RIOT/tests/thread_msg_block_race/tests/01-run.py", line 22, in <module>
    sys.exit(run(testfunc, timeout=10))
  File "/home/harter/work/git/RIOT/dist/pythonlibs/testrunner/__init__.py", line 23, in run
    testfunc(child)
  File "/home/harter/work/git/RIOT/tests/thread_msg_block_race/tests/01-run.py", line 18, in testfunc
    assert(res == 0)
AssertionError
/home/harter/work/git/RIOT/tests/thread_msg_block_race/../../Makefile.include:576: recipe for target 'test' failed
make: *** [test] Error 1

miri64 · 2019-03-28T18:20:05Z

Travis just does the same tests as Murdock but is stuck again for some reason...

miri64 requested review from cladmi and kaspar030 January 30, 2019 18:18

miri64 force-pushed the tests/new/i10881 branch from 7c5175e to 15a7b82 Compare January 30, 2019 18:19

aabadie reviewed Jan 30, 2019

View reviewed changes

tests/thread_msg_block_race/tests/01-run.py Show resolved Hide resolved

miri64 mentioned this pull request Jan 30, 2019

core: msg_receive() on native sometimes returns without msg being re-set #10881

Closed

kaspar030 mentioned this pull request Feb 1, 2019

cpu/native: fix race in thread_yield_higher() #10891

Merged

miri64 mentioned this pull request Feb 1, 2019

cpu/native: fix race in thread_yield_higher() [backport 2019.01] #10921

Merged

miri64 force-pushed the tests/new/i10881 branch from 9dd3bf1 to 7d5a703 Compare February 1, 2019 15:30

miri64 added this to the Release 2019.01 milestone Feb 1, 2019

RIOT-OS deleted a comment Feb 2, 2019

miri64 mentioned this pull request Feb 4, 2019

Release 2019.01 - RC2 RIOT-OS/Release-Specs#98

Closed

71 tasks

aabadie modified the milestones: Release 2019.01, Release 2019.04 Feb 12, 2019

miri64 mentioned this pull request Feb 13, 2019

Release 2019.01 - RC3 RIOT-OS/Release-Specs#105

Closed

75 tasks

kaspar030 reviewed Feb 13, 2019

View reviewed changes

tests/thread_msg_block_race/main.c Outdated Show resolved Hide resolved

kaspar030 reviewed Feb 13, 2019

View reviewed changes

tests/thread_msg_block_race/main.c Outdated Show resolved Hide resolved

kaspar030 reviewed Feb 13, 2019

View reviewed changes

tests/thread_msg_block_race/main.c Show resolved Hide resolved

RIOT-OS deleted a comment Feb 16, 2019

cladmi approved these changes Mar 28, 2019

View reviewed changes

miri64 force-pushed the tests/new/i10881 branch from 794daa1 to 9f4ac4a Compare March 28, 2019 14:21

tests: add tests to reproduce RIOT-OS#10881

3b95ac0

miri64 force-pushed the tests/new/i10881 branch from 155f8af to 3b95ac0 Compare March 28, 2019 17:41

miri64 merged commit 7220ed2 into RIOT-OS:master Mar 28, 2019

miri64 deleted the tests/new/i10881 branch March 28, 2019 18:20

Conversation

miri64 commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

miri64 commented Jan 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miri64 commented Jan 30, 2019

Uh oh!

miri64 commented Jan 31, 2019

Uh oh!

aabadie commented Jan 31, 2019

Uh oh!

miri64 commented Jan 31, 2019

Uh oh!

miri64 commented Feb 1, 2019

Uh oh!

miri64 commented Feb 1, 2019

Uh oh!

miri64 commented Feb 1, 2019

Uh oh!

miri64 commented Feb 13, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miri64 commented Feb 13, 2019

Uh oh!

miri64 commented Mar 24, 2019

Uh oh!

miri64 commented Mar 27, 2019

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

miri64 commented Mar 28, 2019

Uh oh!

miri64 commented Mar 28, 2019

Uh oh!

cladmi commented Mar 28, 2019

Uh oh!

cladmi commented Mar 28, 2019

Uh oh!

miri64 commented Mar 28, 2019

Uh oh!

cladmi commented Mar 28, 2019

Uh oh!

miri64 commented Mar 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miri64 commented Jan 30, 2019 •

edited

Loading

miri64 commented Jan 30, 2019 •

edited

Loading