Skip to content

Commit

Permalink
assertIsCurrent(WorkQueue::main()) will never assert
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261139
rdar://114970446

Reviewed by Kimmo Kinnunen.

Initialise the WorkQueue threadID to 1 when creating WorkQueue::main. We can
make this assumption as the first thread ever created is the Main Thread.
Simplify WorkQueue::threadLikeAssertion() now that WorkQueue::m_threadID
is always set to either the RunLoop's uid or the dispatch_queue sequence id.

Add API test.

* Source/WTF/wtf/ThreadAssertions.h:
* Source/WTF/wtf/WorkQueue.cpp:
(WTF::WorkQueue::threadLikeAssertion const):
* Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp:
(WTF::WorkQueueBase::WorkQueueBase):
(WTF::WorkQueueBase::platformInitialize):
(WTF::WorkQueue::threadLikeAssertion const): Deleted.
* Source/WTF/wtf/generic/WorkQueueGeneric.cpp:
(WTF::WorkQueueBase::WorkQueueBase):
(WTF::WorkQueue::threadLikeAssertion const): Deleted.
* Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp:
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/267640@main
  • Loading branch information
jyavenard committed Sep 5, 2023
1 parent aaec31b commit 0945d4c
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 19 deletions.
3 changes: 2 additions & 1 deletion Source/WTF/wtf/ThreadAssertions.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class ThreadLike {
WTF_EXPORT_PRIVATE static uint32_t currentSequence();

protected:
static constexpr uint32_t mainThreadID { 1 };
static std::atomic<uint32_t> s_uid;
static ThreadLikeAssertion createThreadLikeAssertion(uint32_t);
};
Expand Down Expand Up @@ -99,9 +100,9 @@ class WTF_CAPABILITY("is current") ThreadLikeAssertion {
ThreadLikeAssertion& operator=(ThreadLikeAssertion&&);

void reset() { *this = currentThreadLike; }
bool isCurrent() const; // Public as used in API tests.
private:
constexpr ThreadLikeAssertion(uint32_t uid);
bool isCurrent() const;
#if ASSERT_ENABLED
uint32_t m_uid;
#endif
Expand Down
5 changes: 5 additions & 0 deletions Source/WTF/wtf/WorkQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,11 @@ void WorkQueue::assertIsCurrent() const
{
WTF::assertIsCurrent(threadLikeAssertion());
}

ThreadLikeAssertion WorkQueue::threadLikeAssertion() const
{
return createThreadLikeAssertion(m_threadID);
}
#endif

ConcurrentWorkQueue::ConcurrentWorkQueue(const char* name, QOS qos)
Expand Down
16 changes: 5 additions & 11 deletions Source/WTF/wtf/cocoa/WorkQueueCocoa.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ void WorkQueueBase::dispatchSync(Function<void()>&& function)

WorkQueueBase::WorkQueueBase(OSObjectPtr<dispatch_queue_t>&& dispatchQueue)
: m_dispatchQueue(WTFMove(dispatchQueue))
#if ASSERT_ENABLED
, m_threadID(mainThreadID)
#endif
{
}

Expand All @@ -91,7 +94,8 @@ void WorkQueueBase::platformInitialize(const char* name, Type type, QOS qos)
// We use s_uid to generate the id so that WorkQueues and Threads share the id namespace.
// This makes it possible to assert that code runs in the expected sequence, regardless of if it is
// in a thread or a work queue.
dispatch_queue_set_specific(m_dispatchQueue.get(), &s_uid, reinterpret_cast<void*>(static_cast<uintptr_t>(++s_uid)), nullptr);
m_threadID = ++s_uid;
dispatch_queue_set_specific(m_dispatchQueue.get(), &s_uid, reinterpret_cast<void*>(m_threadID), nullptr);
#endif
}

Expand All @@ -105,16 +109,6 @@ WorkQueue::WorkQueue(MainTag)
// Note: for main work queue we do not create a sequence id, the main thread id will be used.
}

#if ASSERT_ENABLED
ThreadLikeAssertion WorkQueue::threadLikeAssertion() const
{
auto sequenceID = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(dispatch_queue_get_specific(m_dispatchQueue.get(), &s_uid)));
if (!sequenceID)
sequenceID = Thread::current().uid(); // Main thread sequence id.
return createThreadLikeAssertion(sequenceID);
}
#endif

void ConcurrentWorkQueue::apply(size_t iterations, WTF::Function<void(size_t index)>&& function)
{
dispatch_apply(iterations, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), makeBlockPtr([function = WTFMove(function)](size_t index) {
Expand Down
10 changes: 3 additions & 7 deletions Source/WTF/wtf/generic/WorkQueueGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ namespace WTF {

WorkQueueBase::WorkQueueBase(RunLoop& runLoop)
: m_runLoop(&runLoop)
#if ASSERT_ENABLED
, m_threadID(mainThreadID)
#endif
{
}

Expand Down Expand Up @@ -82,11 +85,4 @@ WorkQueue::WorkQueue(MainTag)
{
}

#if ASSERT_ENABLED
ThreadLikeAssertion WorkQueue::threadLikeAssertion() const
{
return createThreadLikeAssertion(m_threadID);
}
#endif

}
21 changes: 21 additions & 0 deletions Tools/TestWebKitAPI/Tests/WTF/WorkQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,4 +354,25 @@ TEST(WTF_WorkQueue, ThreadSafetyAnalysisAssertIsCurrentWorks)
EXPECT_EQ(iterationCount + 1, holder.result);
}

#if ASSERT_ENABLED
#define MAYBE_MainWorkQueueIsCurrent MainWorkQueueIsCurrent
#else
#define MAYBE_MainWorkQueueIsCurrent DISABLED_MainWorkQueueIsCurrent
#endif
TEST(WTF_WorkQueue, MAYBE_MainWorkQueueIsCurrent)
{
assertIsCurrent(WorkQueue::main());
auto queue = WorkQueue::create("com.apple.WebKit.Test.MainWorkQueueIsCurrent");
queue->dispatch([] {
auto threadLikeAssertion = WorkQueue::main().threadLikeAssertion();
EXPECT_FALSE(threadLikeAssertion.isCurrent());
threadLikeAssertion.reset(); // To prevent assertion in ThreadLikeAssertion destructor.
});
queue->dispatchSync([] {
auto threadLikeAssertion = WorkQueue::main().threadLikeAssertion();
EXPECT_FALSE(threadLikeAssertion.isCurrent());
threadLikeAssertion.reset(); // To prevent assertion in ThreadLikeAssertion destructor.
});
}

} // namespace TestWebKitAPI

0 comments on commit 0945d4c

Please sign in to comment.