This is an archive of the discontinued LLVM Phabricator instance.

Skip lldb-mi tests on FreeBSD for now
AbandonedPublic

Authored by emaste on Feb 18 2015, 8:48 AM.

Details

Reviewers
ki.stfu
Summary

Thread race conditions in lldb-mi cause regular failures. Skip rather than xfail as otherwise each one has to wait for timeout.

Diff Detail

Event Timeline

emaste updated this revision to Diff 20190.Feb 18 2015, 8:48 AM
emaste retitled this revision from to Skip lldb-mi tests on FreeBSD for now.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added a subscriber: Unknown Object (MLST).

I think it will be easier to control if you skip each lldb-mi test using @ skipIfFreeBSD annotation.

I think it will be easier to control if you skip each lldb-mi test using @ skipIfFreeBSD annotation.

I tried adding @skipIfFreeBSD on each one as it failed, but the failures are inconsistent and different ones come up on subsequent runs. It seems like it will just be a lot of churn for no real benefit to go that way.

This approach makes it easy to run the lldb-mi tests with +m when we think it's worthwhile without having to back out all of the individual annotations each time.

ki.stfu requested changes to this revision.Feb 18 2015, 9:46 AM
ki.stfu added a reviewer: ki.stfu.
ki.stfu added a subscriber: zturner.

I think it will be easier to control if you skip each lldb-mi test using @ skipIfFreeBSD annotation.

I tried adding @skipIfFreeBSD on each one as it failed, but the failures are inconsistent and different ones come up on subsequent runs. It seems like it will just be a lot of churn for no real benefit to go that way.

This approach makes it easy to run the lldb-mi tests with +m when we think it's worthwhile without having to back out all of the individual annotations each time.

If it works unstable on FreeBSD you should skip it and later we can re-enable test on FreeBSD when it will be fixed. I don't see a reason to modify dotest.py file.

FYI: @zturner used the same approach in r226614 when he was solving pexpect issue.

This revision now requires changes to proceed.Feb 18 2015, 9:46 AM

If it works unstable on FreeBSD you should skip it and later we can re-enable test on FreeBSD when it will be fixed. I don't see a reason to modify dotest.py file.

I think it's better to make this change here than add 43 decorators across 12 lldb-mi test files

If it works unstable on FreeBSD you should skip it and later we can re-enable test on FreeBSD when it will be fixed. I don't see a reason to modify dotest.py file.

I think it's better to make this change here than add 43 decorators across 12 lldb-mi test files

It's 'sad' that lldb-mi has grown to such an extent, but the designed @ skipIfFreeBSD is better than a hack in dotest.py.

Fwiw I don't feel strongly either way. Right now it seems there's issues
with lldb-mi such that even disabling them individually now, a new test
that gets added might have the same issue, because the issue is not in the
implementation of a particular test or feature, but of the larger lldb-mi
library. So this would still lead to the possibility that any future test
may also have to be disabled, creating more work than necessary.

Of course, the correct way to deal with this is to prioritize finding and
fixing the underlying race conditions. Presumably they affect not just
FreeBSD.

Of course, the correct way to deal with this is to prioritize finding and
fixing the underlying race conditions. Presumably they affect not just
FreeBSD.

Indeed. On FreeBSD we often encounter threading issues earlier or more often than other platforms, for two reasons.

First, our thread library places stricter demands on POSIX compliance of consumers. For example, pthread_rwlock_wrlock and pthread_rwlock_unlock must be called from the same thread, but this was originally violated by LLDB. In practice it seemed to work on OS X and Linux, but did not on FreeBSD (which returns EINVAL from the unlock).

Second, we seem to have wider race windows in certain cases due to differences in scheduling or preemption. Issues that show up under these conditions will also happen on other platforms, perhaps just less frequently.

emaste abandoned this revision.Feb 19 2015, 12:46 PM

Added individual decorators instead and work is in progress to fix the threading issues.

Lldb-mi team,

How stable are your tests on OSX? If you run them 100 times, what are the
results?

Vince