Issue 2339043002: Consistently lock RTCVideoDecoder::state_.

Issue 2339043002: Consistently lock RTCVideoDecoder::state_. (Closed)

Created:
4 years, 3 months ago by Jeffrey Yasskin

Modified:
4 years, 3 months ago

Reviewers:
wuchengli, Pawel Osciak

CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@lkcr

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

Consistently lock RTCVideoDecoder::state_. RTCVideoDecoder::ResetInternal() forgot to lock it, which caused TSan errors in the unittest. Committed: https://crrev.com/896fac40e5465b500d1cb831f3227ab093eed861 Cr-Commit-Position: refs/heads/master@{#418669}

Patch Set 1 #

Created: 4 years, 3 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+3 lines, -1 line)			Patch
	M	content/renderer/media/gpu/rtc_video_decoder.cc	View		1 chunk	+3 lines, -1 line	0 comments	Download

Messages

Total messages: 9 (4 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Jeffrey Yasskin

An example TSan error is in https://build.chromium.org/p/chromium.memory.full/builders/Linux%20TSan%20Tests/builds/1576/steps/content_unittests%20on%20Ubuntu-12.04/logs/stdio: WARNING: ThreadSanitizer: data race (pid=14349) Write of size ...

4 years, 3 months ago (2016-09-14 00:44:58 UTC) #2

An example TSan error is in
https://build.chromium.org/p/chromium.memory.full/builders/Linux%20TSan%20Tes...:

WARNING: ThreadSanitizer: data race (pid=14349)
  Write of size 4 at 0x7d4c00010210 by thread T7:
    #0 content::RTCVideoDecoder::ResetInternal()
content/renderer/media/gpu/rtc_video_decoder.cc:676:14
(content_unittests+0x00000493dc8f)
    #1 Invoke<const base::WeakPtr<content::RTCVideoDecoder> &>
base/bind_internal.h:214:12 (content_unittests+0x000004940577)
    #2 MakeItSo<void (content::RTCVideoDecoder::*const &)(), const
base::WeakPtr<content::RTCVideoDecoder> &> base/bind_internal.h:305
(content_unittests+0x000004940577)
    #3 RunImpl<void (content::RTCVideoDecoder::*const &)(), const
std::__1::tuple<base::WeakPtr<content::RTCVideoDecoder> > &, 0>
base/bind_internal.h:361 (content_unittests+0x000004940577)
    #4 base::internal::Invoker<base::internal::BindState<void
(content::RTCVideoDecoder::*)(), base::WeakPtr<content::RTCVideoDecoder> >, void
()>::Run(base::internal::BindStateBase*) base/bind_internal.h:339
(content_unittests+0x000004940577)
    #5 Run base/callback.h:64:12 (content_unittests+0x000003da4753)
    #6 base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask
const&) base/debug/task_annotator.cc:54 (content_unittests+0x000003da4753)

  Previous read of size 4 at 0x7d4c00010210 by main thread (mutexes: write
M371121784675697128):
    #0 content::RTCVideoDecoder::Decode(webrtc::EncodedImage const&, bool,
webrtc::RTPFragmentationHeader const*, webrtc::CodecSpecificInfo const*, long)
content/renderer/media/gpu/rtc_video_decoder.cc:178:7
(content_unittests+0x00000493c451)
    #1
content::RTCVideoDecoderTest_GetVDAErrorCounterForTesting_Test::TestBody()
content/renderer/media/gpu/rtc_video_decoder_unittest.cc:315:3
(content_unittests+0x00000163edd3)
    #2 HandleExceptionsInMethodIfSupported<testing::Test, void>
testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a5a50c)
    #3 testing::Test::Run() testing/gtest/src/gtest.cc:2474
(content_unittests+0x000003a5a50c)
    #4 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11
(content_unittests+0x000003a5b57d)
    #5 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28
(content_unittests+0x000003a5be56)
    #6 testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4647:43 (content_unittests+0x000003a652c6)
    #7 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool> testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a64c74)
    #8 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255
(content_unittests+0x000003a64c74)
    #9 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46
(content_unittests+0x00000323cb14)

  Location is heap block of size 392 at 0x7d4c00010140 allocated by main thread:
    #0 operator new(unsigned long) <null> (content_unittests+0x000000586a12)
    #1 release buildtools/third_party/libc++/trunk/include/memory:2725:30
(content_unittests+0x00000493bb17)
    #2 content::RTCVideoDecoder::Create(webrtc::VideoCodecType,
media::GpuVideoAcceleratorFactories*)
content/renderer/media/gpu/rtc_video_decoder.cc:139
(content_unittests+0x00000493bb17)
    #3 CreateDecoder
content/renderer/media/gpu/rtc_video_decoder_unittest.cc:110:9
(content_unittests+0x00000163e5ce)
    #4
content::RTCVideoDecoderTest_GetVDAErrorCounterForTesting_Test::TestBody()
content/renderer/media/gpu/rtc_video_decoder_unittest.cc:289
(content_unittests+0x00000163e5ce)
    #5 HandleExceptionsInMethodIfSupported<testing::Test, void>
testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a5a50c)
    #6 testing::Test::Run() testing/gtest/src/gtest.cc:2474
(content_unittests+0x000003a5a50c)
    #7 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11
(content_unittests+0x000003a5b57d)
    #8 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28
(content_unittests+0x000003a5be56)
    #9 testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4647:43 (content_unittests+0x000003a652c6)
    #10 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool> testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a64c74)
    #11 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255
(content_unittests+0x000003a64c74)
    #12 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46
(content_unittests+0x00000323cb14)
    #13 base::TestSuite::Run() base/test/test_suite.cc:246
(content_unittests+0x00000323cb14)
    #14 content::UnitTestTestSuite::Run()
content/public/test/unittest_test_suite.cc:45:23
(content_unittests+0x0000031fe781)
    #15 Invoke<content::UnitTestTestSuite *> base/bind_internal.h:214:12
(content_unittests+0x0000015e4975)
    #16 MakeItSo<int (content::UnitTestTestSuite::*const &)(),
content::UnitTestTestSuite *> base/bind_internal.h:285
(content_unittests+0x0000015e4975)
    #17 RunImpl<int (content::UnitTestTestSuite::*const &)(), const
std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> >
&, 0> base/bind_internal.h:361 (content_unittests+0x0000015e4975)
    #18 base::internal::Invoker<base::internal::BindState<int
(content::UnitTestTestSuite::*)(),
base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int
()>::Run(base::internal::BindStateBase*) base/bind_internal.h:339
(content_unittests+0x0000015e4975)
    #19 Run base/callback.h:64:12 (content_unittests+0x0000032564b3)
    #20 base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int
(), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, int,
int, bool, base::Callback<void (), (base::internal::CopyMode)1,
(base::internal::RepeatMode)1> const&)
base/test/launcher/unit_test_launcher.cc:206 (content_unittests+0x0000032564b3)
    #21 base::LaunchUnitTests(int, char**, base::Callback<int (),
(base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&)
base/test/launcher/unit_test_launcher.cc:445:10
(content_unittests+0x000003256331)
    #22 main content/test/run_all_unittests.cc:24:10
(content_unittests+0x0000015e48ac)

  Mutex M371121784675697128 is already destroyed.

  Thread T7 'vda_thread' (tid=14371, running) created by main thread at:
    #0 pthread_create <null> (content_unittests+0x0000005281b5)
    #1 base::(anonymous namespace)::CreateThread(unsigned long, bool,
base::PlatformThread::Delegate*, base::PlatformThreadHandle*,
base::ThreadPriority) base/threading/platform_thread_posix.cc:110:13
(content_unittests+0x000003d5e557)
    #2 base::PlatformThread::CreateWithPriority(unsigned long,
base::PlatformThread::Delegate*, base::PlatformThreadHandle*,
base::ThreadPriority) base/threading/platform_thread_posix.cc:191:10
(content_unittests+0x000003d5e455)
    #3 base::Thread::StartWithOptions(base::Thread::Options const&)
base/threading/thread.cc:108:15 (content_unittests+0x000003d641d9)
    #4 base::Thread::Start() base/threading/thread.cc:71:10
(content_unittests+0x000003d6400b)
    #5 content::RTCVideoDecoderTest::SetUp()
content/renderer/media/gpu/rtc_video_decoder_unittest.cc:58:5
(content_unittests+0x00000163fb98)
    #6 HandleExceptionsInMethodIfSupported<testing::Test, void>
testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a5a3fd)
    #7 testing::Test::Run() testing/gtest/src/gtest.cc:2470
(content_unittests+0x000003a5a3fd)
    #8 testing::TestInfo::Run() testing/gtest/src/gtest.cc:2656:11
(content_unittests+0x000003a5b57d)
    #9 testing::TestCase::Run() testing/gtest/src/gtest.cc:2774:28
(content_unittests+0x000003a5be56)
    #10 testing::internal::UnitTestImpl::RunAllTests()
testing/gtest/src/gtest.cc:4647:43 (content_unittests+0x000003a652c6)
    #11 HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool> testing/gtest/src/gtest.cc:2458:12 (content_unittests+0x000003a64c74)
    #12 testing::UnitTest::Run() testing/gtest/src/gtest.cc:4255
(content_unittests+0x000003a64c74)
    #13 RUN_ALL_TESTS testing/gtest/include/gtest/gtest.h:2237:46
(content_unittests+0x00000323cb14)
    #14 base::TestSuite::Run() base/test/test_suite.cc:246
(content_unittests+0x00000323cb14)
    #15 content::UnitTestTestSuite::Run()
content/public/test/unittest_test_suite.cc:45:23
(content_unittests+0x0000031fe781)
    #16 Invoke<content::UnitTestTestSuite *> base/bind_internal.h:214:12
(content_unittests+0x0000015e4975)
    #17 MakeItSo<int (content::UnitTestTestSuite::*const &)(),
content::UnitTestTestSuite *> base/bind_internal.h:285
(content_unittests+0x0000015e4975)
    #18 RunImpl<int (content::UnitTestTestSuite::*const &)(), const
std::__1::tuple<base::internal::UnretainedWrapper<content::UnitTestTestSuite> >
&, 0> base/bind_internal.h:361 (content_unittests+0x0000015e4975)
    #19 base::internal::Invoker<base::internal::BindState<int
(content::UnitTestTestSuite::*)(),
base::internal::UnretainedWrapper<content::UnitTestTestSuite> >, int
()>::Run(base::internal::BindStateBase*) base/bind_internal.h:339
(content_unittests+0x0000015e4975)
    #20 Run base/callback.h:64:12 (content_unittests+0x0000032564b3)
    #21 base::(anonymous namespace)::LaunchUnitTestsInternal(base::Callback<int
(), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, int,
int, bool, base::Callback<void (), (base::internal::CopyMode)1,
(base::internal::RepeatMode)1> const&)
base/test/launcher/unit_test_launcher.cc:206 (content_unittests+0x0000032564b3)
    #22 base::LaunchUnitTests(int, char**, base::Callback<int (),
(base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&)
base/test/launcher/unit_test_launcher.cc:445:10
(content_unittests+0x000003256331)
    #23 main content/test/run_all_unittests.cc:24:10
(content_unittests+0x0000015e48ac)

SUMMARY: ThreadSanitizer: data race
content/renderer/media/gpu/rtc_video_decoder.cc:676:14 in
content::RTCVideoDecoder::ResetInternal()

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2339043002/1

4 years, 3 months ago (2016-09-14 19:43:45 UTC) #6

commit-bot: I haz the power

4 years, 3 months ago (2016-09-14 20:57:05 UTC) #9

Message was sent while issue was closed.

Patchset 1 (id:??) landed as
https://crrev.com/896fac40e5465b500d1cb831f3227ab093eed861
Cr-Commit-Position: refs/heads/master@{#418669}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages