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

Remove lazy vmt config proto generation #8

Conversation

zakerinasab
Copy link

No description provided.

@zakerinasab zakerinasab closed this Dec 9, 2019
skyostil pushed a commit to skyostil/perfetto that referenced this pull request Feb 11, 2023
TSAN reported the problem below on a CI run.

There's a possible sequence of events that can lead to this race (I'm
not sure it's the only one):
* The perfetto SDK thread calls the OnStop callback.
* The perfetto SDK thread is now inside WaitableTestEvent::Notify().
* The perfetto SDK thread sets notified_ to true and it unlocks the
  mutex, but does not call `cv_.notify_one()`.
* The main thread executes `WaitableTestEvent::Wait()`, which at this
  point can return.
* The main thread finishes executing the test and destroys the tracing
  sessions, which destroys the WaitableTestEvent and therefore `cv_`.
* The perfetto SDK thread resumes execution and tries to execute
  `cv_.notify_one()`, but that races with `cv_` destructor.

In order to prevent this race, we can just call `cv_.notify_one()` under
the lock. It might be less efficient, but it's correct.

This reverts part of https://r.android.com/1373304

https://ci.perfetto.dev/#!/logs/20221219192758--cls-2345694-4--linux-clang-x86_64-tsan

```
[00:07:40] [ RUN      ] PerfettoApiTest/PerfettoApiTest.TrackEventObserver_TwoDataSources/System
[00:07:40] [901.194] ing_service_impl.cc:980 Configured tracing session 1, #sources:1, duration:500 ms, #buffers:1, total buffer size:1024 KB, total sessions:1, uid:1337 session name: ""
[00:07:41] [901.697] ng_service_impl.cc:1921 FlushAndDisableTracing(1) done, success=1
[00:07:41] [901.698] ing_service_impl.cc:980 Configured tracing session 2, #sources:1, duration:500 ms, #buffers:1, total buffer size:1024 KB, total sessions:2, uid:1337 session name: ""
[00:07:41] [902.200] ng_service_impl.cc:1921 FlushAndDisableTracing(2) done, success=1
[00:07:42] ==================
[00:07:42] WARNING: ThreadSanitizer: data race (pid=7841)
[00:07:42]   Write of size 8 at 0x7b2000019c40 by main thread:
[00:07:42]     #0 pthread_cond_destroy ??:? (perfetto_integrationtests+0x315f8d) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#1 std::Cr::condition_variable::~condition_variable() ??:? (libc++.so+0xad729) (BuildId: bea413d45487d5c2)
[00:07:42]     catapult-project#2 (anonymous namespace)::PerfettoApiTest::TearDown() api_integrationtest.cc:? (perfetto_integrationtests+0x398609) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#3 testing::Test::Run() ??:? (perfetto_integrationtests+0x8de94f) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#4 testing::TestInfo::Run() ??:? (perfetto_integrationtests+0x8e0a2c) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#5 testing::TestSuite::Run() ??:? (perfetto_integrationtests+0x8e1b14) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#6 testing::internal::UnitTestImpl::RunAllTests() ??:? (perfetto_integrationtests+0x8f9e3f) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#7 testing::UnitTest::Run() ??:? (perfetto_integrationtests+0x8f8d36) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#8 main api_integrationtest_main.cc:? (perfetto_integrationtests+0x4d7aa4) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]
[00:07:42]   Previous read of size 8 at 0x7b2000019c40 by thread T3:
[00:07:42]     #0 pthread_cond_signal ??:? (perfetto_integrationtests+0x315c28) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#1 std::Cr::condition_variable::notify_one() ??:? (libc++.so+0xad479) (BuildId: bea413d45487d5c2)
[00:07:42]     catapult-project#2 void std::Cr::__function::__policy_invoker<void ()>::__call_impl<std::Cr::__function::__default_alloc_func<(anonymous namespace)::PerfettoApiTest::NewTrace(perfetto::protos::gen::TraceConfig const&, perfetto::BackendType, int)::{lambda()catapult-project#1}, void ()> >(std::Cr::__function::__policy_storage const*) api_integrationtest.cc:? (perfetto_integrationtests+0x39ba0a) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#3 perfetto::base::UnixTaskRunner::RunImmediateAndDelayedTask() unix_task_runner.cc:? (perfetto_integrationtests+0x594cfd) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#4 perfetto::base::UnixTaskRunner::Run() unix_task_runner.cc:? (perfetto_integrationtests+0x594133) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#5 perfetto::base::ThreadTaskRunner::RunTaskThread(std::Cr::function<void (perfetto::base::UnixTaskRunner*)>) thread_task_runner.cc:? (perfetto_integrationtests+0x592e4f) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#6 void* std::Cr::__thread_proxy[abi:v160000]<std::Cr::tuple<std::Cr::unique_ptr<std::Cr::__thread_struct, std::Cr::default_delete<std::Cr::__thread_struct> >, void (perfetto::base::ThreadTaskRunner::*)(std::Cr::function<void (perfetto::base::UnixTaskRunner*)>), perfetto::base::ThreadTaskRunner*, std::Cr::function<void (perfetto::base::UnixTaskRunner*)> > >(void*) thread_task_runner.cc:? (perfetto_integrationtests+0x59380b) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]
[00:07:42]   Location is heap block of size 120 at 0x7b2000019c00 allocated by main thread:
[00:07:42]     #0 operator new(unsigned long) ??:? (perfetto_integrationtests+0x3941a7) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#1 (anonymous namespace)::PerfettoApiTest::NewTrace(perfetto::protos::gen::TraceConfig const&, int) api_integrationtest.cc:? (perfetto_integrationtests+0x39b864) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#2 (anonymous namespace)::PerfettoApiTest::NewTraceWithCategories(std::Cr::vector<std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char> >, std::Cr::allocator<std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char> > > >, perfetto::protos::gen::TrackEventConfig, perfetto::protos::gen::TraceConfig) api_integrationtest.cc:? (perfetto_integrationtests+0x3a6385) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#3 (anonymous namespace)::PerfettoApiTest_TrackEventObserver_TwoDataSources_Test::TestBody() api_integrationtest.cc:? (perfetto_integrationtests+0x4a61d2) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#4 testing::Test::Run() ??:? (perfetto_integrationtests+0x8de8c3) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#5 testing::TestInfo::Run() ??:? (perfetto_integrationtests+0x8e0a2c) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#6 testing::TestSuite::Run() ??:? (perfetto_integrationtests+0x8e1b14) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#7 testing::internal::UnitTestImpl::RunAllTests() ??:? (perfetto_integrationtests+0x8f9e3f) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#8 testing::UnitTest::Run() ??:? (perfetto_integrationtests+0x8f8d36) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     #9 main api_integrationtest_main.cc:? (perfetto_integrationtests+0x4d7aa4) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]
[00:07:42]   Thread T3 'TracingMuxer' (tid=7845, running) created by main thread at:
[00:07:42]     #0 pthread_create ??:? (perfetto_integrationtests+0x3147ab) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#1 perfetto::base::ThreadTaskRunner::ThreadTaskRunner(std::Cr::basic_string<char, std::Cr::char_traits<char>, std::Cr::allocator<char> > const&) thread_task_runner.cc:? (perfetto_integrationtests+0x592b80) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#2 perfetto::(anonymous namespace)::PlatformPosix::CreateTaskRunner(perfetto::Platform::CreateTaskRunnerArgs const&) platform_posix.cc:? (perfetto_integrationtests+0x8c524a) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#3 perfetto::internal::TracingMuxerImpl::TracingMuxerImpl(perfetto::TracingInitArgs const&) tracing_muxer_impl.cc:? (perfetto_integrationtests+0x5a731c) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#4 perfetto::internal::TracingMuxerImpl::InitializeInstance(perfetto::TracingInitArgs const&) tracing_muxer_impl.cc:? (perfetto_integrationtests+0x5ad7fb) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#5 perfetto::Tracing::InitializeInternal(perfetto::TracingInitArgs const&) tracing.cc:? (perfetto_integrationtests+0x5c0f0d) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#6 (anonymous namespace)::ConcurrentSessionTest_ConcurrentBackends_Test::TestBody() api_integrationtest.cc:? (perfetto_integrationtests+0x4c3af4) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#7 testing::Test::Run() ??:? (perfetto_integrationtests+0x8de8c3) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     catapult-project#8 testing::TestInfo::Run() ??:? (perfetto_integrationtests+0x8e0a2c) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     #9 testing::TestSuite::Run() ??:? (perfetto_integrationtests+0x8e1b14) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     #10 testing::internal::UnitTestImpl::RunAllTests() ??:? (perfetto_integrationtests+0x8f9e3f) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     #11 testing::UnitTest::Run() ??:? (perfetto_integrationtests+0x8f8d36) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]     #12 main api_integrationtest_main.cc:? (perfetto_integrationtests+0x4d7aa4) (BuildId: 2d1df55c4bda2cdd)
[00:07:42]
[00:07:42] SUMMARY: ThreadSanitizer: data race ??:? in __interceptor_pthread_cond_destroy
[00:07:42] ==================
```

Change-Id: Ic822dba258b81a6c032633cf63763bd578739024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant