Closed Bug 1276156 Opened 4 years ago Closed 4 years ago
deadlock shutting down vie
_encoder in statistics
This may be a major cause of intermittent test-timed-out failures. Looking through the logs of a couple timeouts on Try, I noticed that at least two had the same signature: At least one thread hanging shutting down a vie_encoder thread: (MainThread in this instance:) ProcessThreadImpl::DeRegisterModule() (criticalsection.h:117) ViEEncoder::StopThreadsAndRemoveSharedMembers [vie_encoder.cc:225] ... WebrtcVideoConduit::Destroy [VideoConduit.cpp:506] This waits on the per-thread 'lock_' to be available, which is held while running code for the thread (note). At least one thread in ProcessThreadImpl::Process() and *not* in Wait(), and generally in vie statistics code: ProcessThreadImpl::Process [process_thread_impl.cc:209] VideoCodingModuleImpl::Process [video_coding_impl.cc:100] ... ViECodecImpl::GetSendCodecStatistics [vie_channel_manager.cc:395] *** and in another timeout (android): vcm::VideoSender::Process [video_sender.cc:99] ViEEncoder::SendStatistics [vie_encoder.cc:783] VideoCodecStatistics::OutgoingRate [CodecStatistics.cpp:60] ViECodecImpl::GetSendCodecStatistics [vie_channel_manager.cc:395] *** ViEInputManagerScoped::ViEInputManagerScoped [vie_manager_base.cc:45] RWLockGeneric::AcquireLockShared [rw_lock_generic.cc:62] Note the *** lines This implies a deadlock around ViEManagerScopedBase::ViEManagerScopedBase()/instance_rwlock_.AcquireLockShared() this should only be possible if it's owned exclusively by someone I think. I'll note all uses of ViEManagerWriteScoped() are scopes, and unless it's sitting in them the lock should be released, and I don't see any thread in them.
To avoid deadlocks between DeleteChannel() and Process() code MozReview-Commit-ID: G650SWDH8s0
Attachment #8757492 - Flags: review?(pkerr)
This is the deadlock scenario: [15:15] jesup we're in VideoCodingModuleImpl::Process() (and thus the ProcessThread lock_ is held while we run) [15:15] jesup on MainThread, we delete the VideoConduit [15:16] jesup which calls DeleteChannel, which grabs an exclusive lock on the ViEManager (ViEManagerWriteScoped wl ((this)) [15:17] jesup then ::Process tries to do ViECodecImpl::GetSendCodecStatistics() which grabs the lock in readonly mode. Since it's held for write, that waits. [15:22] jesup and then DeleteChannel eventually calls ViEEncoder::StopThreadsAndRemoveSharedMembers [vie_encoder.cc:225] which calls ProcessThreadImpl::DeRegisterModule() (criticalsection.h:117) which needs the lock_ -- which is held by the thread waiting on the channel lock [15:22] jesup Boom, deadlock Solution is to stop the threads *before* taking the exclusive lock which can deadlock us.
Note: here are the two other hits we had before that led me to find this: Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=35ca6f3e3049e22cb707dac332de5f7f726fadb1 https://treeherder.mozilla.org/logviewer.html#?job_id=21535308&repo=try https://treeherder.mozilla.org/logviewer.html#?job_id=21534169&repo=try
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/68eb3a876ea5 Stop encoder ProcessThreads before deleting the channel r=pkerr
You need to log in before you can comment on or make changes to this bug.