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

Do not use REQUIRE() from multiple threads #4079

Open
dthaler opened this issue Dec 9, 2024 · 0 comments
Open

Do not use REQUIRE() from multiple threads #4079

dthaler opened this issue Dec 9, 2024 · 0 comments
Assignees
Labels
bug Something isn't working P2 tests triaged Discussed in a triage meeting
Milestone

Comments

@dthaler
Copy link
Collaborator

dthaler commented Dec 9, 2024

Describe the bug

Per https://github.com/catchorg/Catch2/blob/devel/docs/limitations.md#thread-safe-assertions
thread-safe REQUIRE() is not supported by catch2.

We currently have some tests, such as "sock_addr_invoke_concurrent1" in netebpfext_unit.cpp,
"multi_attach_concurrency_test1" in socket_tests.cpp, etc. that incorrectly call REQUIRE() in multiple concurrent threads,
which is not safe to do with catch2, as noted at the above link. This is by design since it says
"We currently do not plan to support thread-safe assertions."

I previously filed catchorg/Catch2#2935 but above link was the answer.

As such, we need to fix ebpf-for-windows to not do so. An alternative might be to set some global variable and exit the thread,
then do REQUIRE() back in the main test function once the threads exit.

OS information

No response

Steps taken to reproduce bug

Upgrade catch2 to latest.

Expected behavior

Upgrading catch2 should succeed.

Actual outcome

https://github.com/microsoft/ebpf-for-windows/actions/runs/12214862186/job/34076302844

Additional details

No response

@dthaler dthaler added bug Something isn't working tests labels Dec 9, 2024
@Alan-Jowett Alan-Jowett added the P2 label Dec 9, 2024
@Alan-Jowett Alan-Jowett added this to the 2501 milestone Dec 9, 2024
@Alan-Jowett Alan-Jowett added the triaged Discussed in a triage meeting label Dec 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P2 tests triaged Discussed in a triage meeting
Projects
None yet
Development

No branches or pull requests

3 participants