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

Fix IPC tests in debug asan builds #9710

Conversation

achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Feb 6, 2023

49c6254

Fix IPC tests in debug asan builds
https://bugs.webkit.org/show_bug.cgi?id=251813

Reviewed by Kimmo Kinnunen.

There were two issues:
1. workQueueWait was referenced after its stack scope during test tear-down.
   Make it static to extend its scope past the '}'
2. ReceiveAlreadyInvalidatedClientNoAssert hit a stack overflow because asan
   uses a little more stack space.  Just reduce 1000 iterations to 800 to
   make that test also run successfully.

* Tools/TestWebKitAPI/Tests/IPC/ConnectionTests.cpp:
(TestWebKitAPI::TEST_P):
* Tools/TestWebKitAPI/Tests/IPC/StreamConnectionTests.cpp:
(TestWebKitAPI::TEST_P):

Canonical link: https://commits.webkit.org/260099@main

ba191c9

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe   πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl   πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac   πŸ§ͺ gtk-wk2
  πŸ§ͺ api-ios   πŸ§ͺ mac-wk1   πŸ§ͺ api-gtk
  πŸ›  tv   πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim   πŸ§ͺ mac-AS-debug-wk2
  πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch-sim

@achristensen07 achristensen07 self-assigned this Feb 6, 2023
@achristensen07 achristensen07 added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label Feb 6, 2023
Copy link
Contributor

@kkinnunen-apple kkinnunen-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

@@ -390,7 +390,7 @@ TEST_P(StreamMessageTest, SendAsyncReplyCancel)
auto cleanup = localReferenceBarrier();

std::atomic<bool> waiting;
BinarySemaphore workQueueWait;
static BinarySemaphore workQueueWait;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem exists with the waiting variable.

This could be instead:

TEST_P(StreamMessageTest, SendAsyncReplyCancel)
{
    auto cleanup = localReferenceBarrier();

@achristensen07 achristensen07 force-pushed the eng/Fix-IPC-tests-in-debug-asan-builds branch from 04628c7 to ba191c9 Compare February 10, 2023 00:25
@achristensen07 achristensen07 added the merge-queue Applied to send a pull request to merge-queue label Feb 10, 2023
https://bugs.webkit.org/show_bug.cgi?id=251813

Reviewed by Kimmo Kinnunen.

There were two issues:
1. workQueueWait was referenced after its stack scope during test tear-down.
   Make it static to extend its scope past the '}'
2. ReceiveAlreadyInvalidatedClientNoAssert hit a stack overflow because asan
   uses a little more stack space.  Just reduce 1000 iterations to 800 to
   make that test also run successfully.

* Tools/TestWebKitAPI/Tests/IPC/ConnectionTests.cpp:
(TestWebKitAPI::TEST_P):
* Tools/TestWebKitAPI/Tests/IPC/StreamConnectionTests.cpp:
(TestWebKitAPI::TEST_P):

Canonical link: https://commits.webkit.org/260099@main
@webkit-commit-queue
Copy link
Collaborator

Committed 260099@main (49c6254): https://commits.webkit.org/260099@main

Reviewed commits have been landed. Closing PR #9710 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 49c6254 into WebKit:main Feb 10, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
4 participants