Skip to content

Commit 752da6a

Browse files
sherry-yuansherry-yuan
committed
Fix TSAN data races
------------------------------------------------------ As found in #71, tsan reported several issues in the acl_test Currently when release command queue are called (which frees the memory) subsequent still checks the values at the same address which cause tsan to report heap-use-after-free. The test after_context_release are moved upward to be the first test because ctest execute the test in reverse order of how they appear in the cpp file. Given after_context_release will release the command queue, they should be run last, i.e place at the top in the test.cpp file
1 parent 8cd410f commit 752da6a

File tree

1 file changed

+57
-70
lines changed

1 file changed

+57
-70
lines changed

‎test/acl_command_queue_test.cpp

Lines changed: 57 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,63 @@ static void CL_CALLBACK notify_me(const char *errinfo, const void *private_info,
8080
private_info = private_info; // avoid warning on windows
8181
}
8282

83+
MT_TEST(acl_command_queue, after_context_release) {
84+
acl_set_allow_invalid_type<cl_command_queue>(1);
85+
// OpenCL 1.0 conformance_test_multiples will release the command queues
86+
// after the underlying contexts are gone.
87+
88+
cl_int status;
89+
cl_context context = clCreateContext(0, 1, &m_device[0], 0, 0, &status);
90+
CHECK_EQUAL(CL_SUCCESS, status);
91+
ACL_LOCKED(CHECK(acl_context_is_valid(context)));
92+
93+
status = 42;
94+
cl_command_queue cq0 = clCreateCommandQueue(context, m_device[0], 0, &status);
95+
CHECK(cq0);
96+
CHECK_EQUAL(CL_SUCCESS, status);
97+
98+
status = 95;
99+
cl_command_queue cq1 = clCreateCommandQueue(context, m_device[0], 0, &status);
100+
CHECK(cq1);
101+
CHECK_EQUAL(CL_SUCCESS, status);
102+
103+
// wait until all threads have their context and command queues before
104+
// continuing so that releasing them below won't cause them to be
105+
// immediately reused for another thread
106+
syncThreads();
107+
108+
// Early release of the context!
109+
CHECK_EQUAL(CL_SUCCESS, clReleaseContext(context));
110+
111+
CHECK(cq0);
112+
CHECK_EQUAL(1, acl_ref_count(cq0));
113+
CHECK(cq1);
114+
CHECK_EQUAL(1, acl_ref_count(cq1));
115+
116+
// Should be able to clFinish if there are no commands enqueued.
117+
CHECK_EQUAL(CL_SUCCESS, clFinish(cq0));
118+
CHECK_EQUAL(CL_SUCCESS, clFinish(cq1));
119+
// Should be able to clFlush if there are no commands enqueued.
120+
CHECK_EQUAL(CL_SUCCESS, clFlush(cq0));
121+
CHECK_EQUAL(CL_SUCCESS, clFlush(cq1));
122+
123+
// Should be able to retain....
124+
CHECK_EQUAL(CL_SUCCESS, clRetainCommandQueue(cq0));
125+
CHECK_EQUAL(CL_SUCCESS, clRetainCommandQueue(cq1));
126+
CHECK_EQUAL(2, acl_ref_count(cq0));
127+
CHECK_EQUAL(2, acl_ref_count(cq1));
128+
129+
// Should be able to release...
130+
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq0));
131+
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq1));
132+
CHECK_EQUAL(1, acl_ref_count(cq0));
133+
CHECK_EQUAL(1, acl_ref_count(cq1));
134+
135+
// Should be able to release all the way.
136+
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq0));
137+
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq1));
138+
}
139+
83140
MT_TEST(acl_command_queue, create) {
84141
acl_set_allow_invalid_type<cl_command_queue>(1);
85142
cl_int status;
@@ -387,8 +444,6 @@ MT_TEST(acl_command_queue, create_with_properties) {
387444
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq));
388445
CHECK_EQUAL(1, acl_ref_count(cq));
389446
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq));
390-
CHECK_EQUAL(0, acl_ref_count(cq));
391-
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(cq)));
392447

393448
// wait until all threads do their checks on the 0-ref-count command
394449
// queue before starting the next iteration of the loop and creating new
@@ -421,8 +476,6 @@ MT_TEST(acl_command_queue, create_with_properties) {
421476
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq));
422477
CHECK_EQUAL(1, acl_ref_count(cq));
423478
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq));
424-
CHECK_EQUAL(0, acl_ref_count(cq));
425-
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(cq)));
426479

427480
// wait until all threads do their checks on the 0-ref-count command
428481
// queue before starting the next iteration of the loop and creating new
@@ -619,72 +672,6 @@ MT_TEST(acl_command_queue, set_prop) {
619672
}
620673
#endif
621674

622-
MT_TEST(acl_command_queue, after_context_release) {
623-
acl_set_allow_invalid_type<cl_command_queue>(1);
624-
// OpenCL 1.0 conformance_test_multiples will release the command queues
625-
// after the underlying contexts are gone.
626-
627-
cl_int status;
628-
cl_context context = clCreateContext(0, 1, &m_device[0], 0, 0, &status);
629-
CHECK_EQUAL(CL_SUCCESS, status);
630-
ACL_LOCKED(CHECK(acl_context_is_valid(context)));
631-
632-
status = 42;
633-
cl_command_queue cq0 = clCreateCommandQueue(context, m_device[0], 0, &status);
634-
CHECK(cq0);
635-
CHECK_EQUAL(CL_SUCCESS, status);
636-
637-
status = 95;
638-
cl_command_queue cq1 = clCreateCommandQueue(context, m_device[0], 0, &status);
639-
CHECK(cq1);
640-
CHECK_EQUAL(CL_SUCCESS, status);
641-
642-
// wait until all threads have their context and command queues before
643-
// continuing so that releasing them below won't cause them to be
644-
// immediately reused for another thread
645-
syncThreads();
646-
647-
// Early release of the context!
648-
CHECK_EQUAL(CL_SUCCESS, clReleaseContext(context));
649-
650-
CHECK(cq0);
651-
CHECK_EQUAL(1, acl_ref_count(cq0));
652-
CHECK(cq1);
653-
CHECK_EQUAL(1, acl_ref_count(cq1));
654-
655-
// Should be able to clFinish if there are no commands enqueued.
656-
CHECK_EQUAL(CL_SUCCESS, clFinish(cq0));
657-
CHECK_EQUAL(CL_SUCCESS, clFinish(cq1));
658-
// Should be able to clFlush if there are no commands enqueued.
659-
CHECK_EQUAL(CL_SUCCESS, clFlush(cq0));
660-
CHECK_EQUAL(CL_SUCCESS, clFlush(cq1));
661-
662-
// Should be able to retain....
663-
CHECK_EQUAL(CL_SUCCESS, clRetainCommandQueue(cq0));
664-
CHECK_EQUAL(CL_SUCCESS, clRetainCommandQueue(cq1));
665-
CHECK_EQUAL(2, acl_ref_count(cq0));
666-
CHECK_EQUAL(2, acl_ref_count(cq1));
667-
668-
// Should be able to release...
669-
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq0));
670-
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq1));
671-
CHECK_EQUAL(1, acl_ref_count(cq0));
672-
CHECK_EQUAL(1, acl_ref_count(cq1));
673-
674-
// Should be able to release all the way.
675-
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq0));
676-
CHECK_EQUAL(CL_SUCCESS, clReleaseCommandQueue(cq1));
677-
CHECK_EQUAL(0, acl_ref_count(cq0));
678-
CHECK_EQUAL(0, acl_ref_count(cq1));
679-
680-
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(cq0)));
681-
ACL_LOCKED(CHECK(!acl_command_queue_is_valid(cq1)));
682-
683-
// And once it's gone, it's gone.
684-
CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clReleaseCommandQueue(cq0));
685-
CHECK_EQUAL(CL_INVALID_COMMAND_QUEUE, clReleaseCommandQueue(cq1));
686-
}
687-
688675
// Main Event is in an OOO queue. It has a dependent event in an in-order queue.
689676
// Test that completing the main event, will unblock the dependent event.
690677
MT_TEST(acl_command_queue, mixed_queue_dependencies_1) {

0 commit comments

Comments
 (0)