Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test with thread sanitizer #12

Open
romangg opened this issue Mar 23, 2020 · 5 comments
Open

Test with thread sanitizer #12

romangg opened this issue Mar 23, 2020 · 5 comments

Comments

@romangg
Copy link
Member

romangg commented Mar 23, 2020

Recent CI fails because of data races under load showed that there were still many tests not behaving well in regards to thread safety (for example at merge of !5 and !6) and potentially there are many more.

While one can fix them one by one whenever a pipeline fails it would be better to check with a thread sanitizer. This would also prevent new data races from creeping in. A separate build is then needed because the thread sanitizer can't be used together with the others we currently employ.

The issue is: I tried this already and the method to check-wait with a signal spy on a signal being emitted seems to be inherently racy. It's always a read after write until the read is successful. I'm unsure how to solve that. Maybe a helper class with a mutex?

@romangg
Copy link
Member Author

romangg commented Mar 23, 2020

The Plasma Window Model test seems to be especially problematic:

FAIL!  : PlasmaWindowModelTest::testTitle() Compared values are not the same
   Loc: [/home/roman/dev/kde/wrapland/src/autotests/client/test_plasma_window_model.cpp(648)]
FAIL!  : PlasmaWindowModelTest::testAppId() Compared values are not the same
   Loc: [/home/roman/dev/kde/wrapland/src/autotests/client/test_plasma_window_model.cpp(682)]

I could reproduce this locally when running the test through valgrind and having stress running for some time.

@romangg
Copy link
Member Author

romangg commented Mar 23, 2020

Removing the discussion label again and moving forward to execute. After all the goal is clear: test with thread sanitizer. If this is realized in the end by introducing mutex around every spy or something else is a technical implementation detail and if this needs discussion the issue can be moved back.

@romangg
Copy link
Member Author

romangg commented Mar 31, 2020

This should help to find the last remaining leaks that only show very sporadically on CI and can not reproduced locally. For example: https://gitlab.com/kwinft/wrapland/-/jobs/492708632

@romangg
Copy link
Member Author

romangg commented Apr 1, 2020

mentioned in commit e44986d

@romangg
Copy link
Member Author

romangg commented Apr 1, 2020

mentioned in commit c2065ba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant