gecko-dev/gfx/thebes/VsyncSource.cpp
Markus Stange 213f2aaa74 Bug 1781167 - Allow stacking calls to Add/RemoveVsyncDispatcher so that we survive the sequence Add,Add,Remove. r=jrmuizel
This fixes a bug which caused Firefox windows to become frozen after some time.

Full credit goes to Susan and RandyS for bisecting the regressor of this bug, and
to Jeff DeFouw for debugging the issue and finding the cause.

The bug here is a "state race" between the VsyncDispatcher state and
the VsyncSource state. Both are protected by locks, and the code that
runs in those locks respectively can see a different orders of invocations.

VsyncDispatcher::UpdateVsyncStatus does this thing where it updates its state inside
a lock, gathers some information, and then calls methods on VsyncSource *outside* the lock.
Since it calls those methods outside the lock, these calls can end up being executed
in a different order than the state changes were observed inside the lock.

Here's the bad scenario in detail, with the same VsyncDispatcher being used from
two different threads, turning a Remove,Add into an Add,Remove:

```
Thread A                                       Thread B

VsyncDispatcher::UpdateVsync
 |
 |----> Enter VsyncDispatcher lock
 |    |                                         VsyncDispatcher::UpdateVsync
 |    |   state->mIsObservingVsync = false       |
 |    |   (We want to stop listening)            |
 |    |                                          |
 |<---- Exit VsyncDispatcher lock                |
 |                                               |----> Enter VsyncDispatcher lock
 |                                               |    |
 |                                               |    |   state->mIsObservingVsync = true
 |                                               |    |   (We want to start listening)
 |                                               |    |
 |                                               |<----  Exit VsyncDispatcher lock
 |                                               |
 |                                               |----> Enter VsyncSource::AddVsyncDispatcher
 |                                               |    |
 |                                               |    |----> Enter VsyncSource lock
 |                                               |    |    |
 |                                               |    |    |  state->mDispatchers.Contains(aVsyncDispatcher)
 |----> VsyncSource::RemoveVsyncDispatcher       |    |    |  VsyncDispatcher already present in list, not doing anything
 |    |                                          |    |    |
 |    |                                          |    |<---- Exit VsyncSource lock
 |    |                                          |    |
 |    |                                          |<---- Exit VsyncSource::AddVsyncDispatcher
 |    |----> Enter VsyncSource lock
 |    |    |
 |    |    |  Removing aVsyncDispatcher from state->mDispatchers
 |    |    |
 |    |<---- Exit VsyncSource lock
 |    |
 |<---- Exit VsyncSource::AddVsyncDispatcher
```

Now the VsyncDispatcher thinks it is still observing vsync, but it is
no longer registered with the VsyncSource.

This patch makes it so that two calls to AddVsyncDispatcher followed by one call
to RemoveVsyncDispatcher result in the VsyncDispatcher still being registered.
AddVsyncDispatcher is no longer idempotent.

Differential Revision: https://phabricator.services.mozilla.com/D162760
2022-11-22 23:46:44 +00:00

161 lines
4.6 KiB
C++

/* -*- Mode: C++; tab-width: 20; indent-tabs-mode: nil; c-basic-offset: 2 -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "VsyncSource.h"
#include "nsThreadUtils.h"
#include "nsXULAppAPI.h"
#include "mozilla/VsyncDispatcher.h"
#include "MainThreadUtils.h"
#include "gfxPlatform.h"
#ifdef MOZ_WAYLAND
# include "WaylandVsyncSource.h"
#endif
namespace mozilla {
namespace gfx {
VsyncSource::VsyncSource() : mState("VsyncSource::State") {
MOZ_ASSERT(NS_IsMainThread());
}
VsyncSource::~VsyncSource() { MOZ_ASSERT(NS_IsMainThread()); }
// Called on the vsync thread
void VsyncSource::NotifyVsync(const TimeStamp& aVsyncTimestamp,
const TimeStamp& aOutputTimestamp) {
VsyncId vsyncId;
nsTArray<DispatcherRefWithCount> dispatchers;
{
auto state = mState.Lock();
vsyncId = state->mVsyncId.Next();
dispatchers = state->mDispatchers.Clone();
state->mVsyncId = vsyncId;
}
// Notify our listeners, outside of the lock.
const VsyncEvent event(vsyncId, aVsyncTimestamp, aOutputTimestamp);
for (const auto& dispatcher : dispatchers) {
dispatcher.mDispatcher->NotifyVsync(event);
}
}
void VsyncSource::AddVsyncDispatcher(VsyncDispatcher* aVsyncDispatcher) {
MOZ_ASSERT(aVsyncDispatcher);
{
auto state = mState.Lock();
// Find the dispatcher in mDispatchers. If it is already present, increment
// the count. If not, add it with a count of 1.
bool found = false;
for (auto& dispatcherRefWithCount : state->mDispatchers) {
if (dispatcherRefWithCount.mDispatcher == aVsyncDispatcher) {
dispatcherRefWithCount.mCount++;
found = true;
break;
}
}
if (!found) {
state->mDispatchers.AppendElement(
DispatcherRefWithCount{aVsyncDispatcher, 1});
}
}
UpdateVsyncStatus();
}
void VsyncSource::RemoveVsyncDispatcher(VsyncDispatcher* aVsyncDispatcher) {
MOZ_ASSERT(aVsyncDispatcher);
{
auto state = mState.Lock();
// Find the dispatcher in mDispatchers. If found, decrement the count.
// If the count becomes zero, remove it from mDispatchers.
for (auto it = state->mDispatchers.begin(); it != state->mDispatchers.end();
++it) {
if (it->mDispatcher == aVsyncDispatcher) {
it->mCount--;
if (it->mCount == 0) {
state->mDispatchers.RemoveElementAt(it);
}
break;
}
}
// In the future we should probably MOZ_RELEASE_ASSERT here that we don't
// try to remove a dispatcher which isn't in mDispatchers.
}
UpdateVsyncStatus();
}
// This is the base class implementation. Subclasses override this method.
TimeDuration VsyncSource::GetVsyncRate() {
// If hardware queries fail / are unsupported, we have to just guess.
return TimeDuration::FromMilliseconds(1000.0 / 60.0);
}
void VsyncSource::UpdateVsyncStatus() {
if (!NS_IsMainThread()) {
NS_DispatchToMainThread(NS_NewRunnableFunction(
"VsyncSource::UpdateVsyncStatus",
[self = RefPtr{this}] { self->UpdateVsyncStatus(); }));
return;
}
MOZ_ASSERT(NS_IsMainThread());
// WARNING: This function SHOULD NOT BE CALLED WHILE HOLDING LOCKS
// NotifyVsync grabs a lock to dispatch vsync events
// When disabling vsync, we wait for the underlying thread to stop on some
// platforms We can deadlock if we wait for the underlying vsync thread to
// stop while the vsync thread is in NotifyVsync.
bool enableVsync = false;
{ // scope lock
auto state = mState.Lock();
enableVsync = !state->mDispatchers.IsEmpty();
}
if (enableVsync) {
EnableVsync();
} else {
DisableVsync();
}
if (IsVsyncEnabled() != enableVsync) {
NS_WARNING("Vsync status did not change.");
}
}
// static
Maybe<TimeDuration> VsyncSource::GetFastestVsyncRate() {
Maybe<TimeDuration> retVal;
if (!gfxPlatform::Initialized()) {
return retVal;
}
RefPtr<VsyncDispatcher> vsyncDispatcher =
gfxPlatform::GetPlatform()->GetGlobalVsyncDispatcher();
RefPtr<VsyncSource> vsyncSource = vsyncDispatcher->GetCurrentVsyncSource();
if (vsyncSource->IsVsyncEnabled()) {
retVal.emplace(vsyncSource->GetVsyncRate());
#ifdef MOZ_WAYLAND
Maybe<TimeDuration> waylandRate = WaylandVsyncSource::GetFastestVsyncRate();
if (waylandRate) {
if (!retVal) {
retVal.emplace(*waylandRate);
} else if (*waylandRate < *retVal) {
retVal = waylandRate;
}
}
#endif
}
return retVal;
}
} // namespace gfx
} // namespace mozilla