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.

Sign up for GitHub

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

Jump to bottom

obs-browser: prevent race condition in CEF paint pipeline #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

obs-browser: prevent race condition in CEF paint pipeline #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 30 additions & 20 deletions browser-client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ bool BrowserClient::OnProcessMessageReceived(
const std::string &name = message->GetName();
Json json;

if (!bs) {
BrowserSource* source = bs;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of doing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen crashes in the wild where !bs check was completed successfully but crashes downstream on bs-> usage. This suggests bs might be invalidated in a parallel thread right after the !bs check completed.


if (!source) {
return false;
}

Expand Down Expand Up @@ -144,11 +146,13 @@ bool BrowserClient::GetViewRect(
CefRefPtr<CefBrowser>,
CefRect &rect)
{
if (!bs) {
BrowserSource* source = bs;

if (!source) {
return false;
}

rect.Set(0, 0, bs->width, bs->height);
rect.Set(0, 0, source->width, source->height);
return true;
}

Expand All @@ -170,28 +174,30 @@ void BrowserClient::OnPaint(
}
#endif

if (!bs) {
BrowserSource* source = bs;

if (!source) {
return;
}

if (bs->width != width || bs->height != height) {
if (source->width != width || source->height != height) {
obs_enter_graphics();
bs->DestroyTextures();
source->DestroyTextures();
obs_leave_graphics();
}

if (!bs->texture && width && height) {
if (!source->texture && width && height) {
obs_enter_graphics();
bs->texture = gs_texture_create(
source->texture = gs_texture_create(
width, height, GS_BGRA, 1,
(const uint8_t **)&buffer,
GS_DYNAMIC);
bs->width = width;
bs->height = height;
source->width = width;
source->height = height;
obs_leave_graphics();
} else {
obs_enter_graphics();
gs_texture_set_image(bs->texture,
gs_texture_set_image(source->texture,
(const uint8_t *)buffer,
width * 4, false);
obs_leave_graphics();
Expand All @@ -205,7 +211,9 @@ void BrowserClient::OnAcceleratedPaint(
const RectList &,
void *shared_handle)
{
if (!bs) {
BrowserSource* source = bs;

if (!source) {
return;
}

Expand All @@ -215,8 +223,8 @@ void BrowserClient::OnAcceleratedPaint(
gs_texture_destroy(texture);
texture = nullptr;
#endif
gs_texture_destroy(bs->texture);
bs->texture = nullptr;
gs_texture_destroy(source->texture);
source->texture = nullptr;

#if USE_TEXTURE_COPY
texture = gs_texture_open_shared(
Expand All @@ -226,9 +234,9 @@ void BrowserClient::OnAcceleratedPaint(
uint32_t cy = gs_texture_get_height(texture);
gs_color_format format = gs_texture_get_color_format(texture);

bs->texture = gs_texture_create(cx, cy, format, 1, nullptr, 0);
source->texture = gs_texture_create(cx, cy, format, 1, nullptr, 0);
#else
bs->texture = gs_texture_open_shared(
source->texture = gs_texture_open_shared(
(uint32_t)(uintptr_t)shared_handle);
#endif
obs_leave_graphics();
Expand All @@ -237,9 +245,9 @@ void BrowserClient::OnAcceleratedPaint(
}

#if USE_TEXTURE_COPY
if (texture && bs->texture) {
if (texture && source->texture) {
obs_enter_graphics();
gs_copy_texture(bs->texture, texture);
gs_copy_texture(source->texture, texture);
obs_leave_graphics();
}
#endif
Expand All @@ -251,12 +259,14 @@ void BrowserClient::OnLoadEnd(
CefRefPtr<CefFrame> frame,
int)
{
if (!bs) {
BrowserSource* source = bs;

if (!source) {
return;
}

if (frame->IsMain()) {
std::string base64EncodedCSS = base64_encode(bs->css);
std::string base64EncodedCSS = base64_encode(source->css);

std::string href;
href += "data:text/css;charset=utf-8;base64,";
Expand Down
2 changes: 1 addition & 1 deletion browser-client.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class BrowserClient : public CefClient,
bool sharing_available = false;

public:
BrowserSource *bs;
BrowserSource *bs = nullptr;
CefRect popupRect;
CefRect originalPopupRect;

Expand Down
74 changes: 35 additions & 39 deletions obs-browser-source.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,27 +125,29 @@ bool BrowserSource::CreateBrowser()
});
}

void BrowserSource::DestroyBrowser(bool async)
void BrowserSource::DestroyBrowser()
{
ExecuteOnBrowser([this] ()
{
CefRefPtr<CefClient> client =
cefBrowser->GetHost()->GetClient();
BrowserClient *bc =
reinterpret_cast<BrowserClient*>(client.get());
if (bc) {
bc->bs = nullptr;
}
if (!cefBrowser.get()) {
return;
}

/*
* This stops rendering
* http://magpcss.org/ceforum/viewtopic.php?f=6&t=12079
* https://bitbucket.org/chromiumembedded/cef/issues/1363/washidden-api-got-broken-on-branch-2062)
*/
cefBrowser->GetHost()->WasHidden(true);
cefBrowser->GetHost()->CloseBrowser(true);
cefBrowser = nullptr;
}, async);
/*
* This stops rendering
* http://magpcss.org/ceforum/viewtopic.php?f=6&t=12079
* https://bitbucket.org/chromiumembedded/cef/issues/1363/washidden-api-got-broken-on-branch-2062)
*/
cefBrowser->GetHost()->WasHidden(true);
cefBrowser->GetHost()->CloseBrowser(true);

CefRefPtr<CefClient> client =
cefBrowser->GetHost()->GetClient();
BrowserClient *bc =
reinterpret_cast<BrowserClient*>(client.get());
if (bc) {
bc->bs = nullptr;
}

cefBrowser = nullptr;
}

void BrowserSource::SendMouseClick(
Expand Down Expand Up @@ -253,11 +255,10 @@ void BrowserSource::SendKeyClick(

void BrowserSource::SetShowing(bool showing)
{
if (!showing) {
ExecuteOnBrowser([this] ()
{
cefBrowser->GetHost()->WasHidden(true);
}, true);
is_showing = showing;

if (!showing && !!cefBrowser.get()) {
cefBrowser->GetHost()->WasHidden(true);
}

if (shutdown_on_invisible) {
Expand All @@ -266,23 +267,18 @@ void BrowserSource::SetShowing(bool showing)
} else {
DestroyBrowser(true);
}
} else {
ExecuteOnBrowser([this, showing] ()
{
CefRefPtr<CefProcessMessage> msg =
CefProcessMessage::Create("Visibility");
CefRefPtr<CefListValue> args = msg->GetArgumentList();
args->SetBool(0, showing);
cefBrowser->SendProcessMessage(PID_RENDERER, msg);
}, true);
} else if (!!cefBrowser.get()) {
CefRefPtr<CefProcessMessage> msg =
CefProcessMessage::Create("Visibility");
CefRefPtr<CefListValue> args =
msg->GetArgumentList();
args->SetBool(0, showing);
cefBrowser->SendProcessMessage(PID_RENDERER, msg);
}

if (showing) {
ExecuteOnBrowser([this] ()
{
cefBrowser->GetHost()->WasHidden(false);
cefBrowser->GetHost()->Invalidate(PET_VIEW);
}, true);
if (showing && !!cefBrowser.get()) {
cefBrowser->GetHost()->WasHidden(false);
cefBrowser->GetHost()->Invalidate(PET_VIEW);
}
}

Expand Down
2 changes: 1 addition & 1 deletion obs-browser-source.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ struct BrowserSource {
/* ---------------------------- */

bool CreateBrowser();
void DestroyBrowser(bool async = false);
void DestroyBrowser();
void ExecuteOnBrowser(std::function<void()> func, bool async = false);

/* ---------------------------- */
Expand Down