Skip to content

Core - Cancel pending tasks when browser crashed or v8 context gets released #5145

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions CefSharp.Core.Runtime/Internals/CefFrameWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ Task<JavascriptResponse^>^ CefFrameWrapper::EvaluateScriptAsync(String^ script,
//If we're unable to get the underlying browser/browserhost then return null
if (!browser.get() || !host.get())
{
return nullptr;
return Task::FromException<JavascriptResponse^>(gcnew InvalidOperationException("Browser host not available"));
}

auto client = static_cast<ClientAdapter*>(host->GetClient().get());

auto pendingTaskRepository = client->GetPendingTaskRepository();

//create a new taskcompletionsource
auto idAndComplectionSource = pendingTaskRepository->CreatePendingTask(timeout);
auto idAndComplectionSource = pendingTaskRepository->CreatePendingTask(Identifier, timeout);

if (useImmediatelyInvokedFuncExpression)
{
Expand Down
14 changes: 11 additions & 3 deletions CefSharp.Core.Runtime/Internals/ClientAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,8 @@ namespace CefSharp

void ClientAdapter::OnRenderProcessTerminated(CefRefPtr<CefBrowser> browser, TerminationStatus status, int errorCode, const CefString& errorString)
{
_pendingTaskRepository->CancelPendingTasks();

auto handler = _browserControl->RequestHandler;

if (handler != nullptr)
Expand Down Expand Up @@ -1382,9 +1384,13 @@ namespace CefSharp
//we get here, only continue if we have a valid frame reference
if (frame.get() && frame->IsValid())
{
auto frameId = StringUtils::ToClr(frame->GetIdentifier());

_pendingTaskRepository->CancelPendingTasks(frameId);

if (frame->IsMain())
{
_browserControl->SetCanExecuteJavascriptOnMainFrame(StringUtils::ToClr(frame->GetIdentifier()), false);
_browserControl->SetCanExecuteJavascriptOnMainFrame(frameId, false);
}

auto handler = _browserControl->RenderProcessMessageHandler;
Expand Down Expand Up @@ -1475,14 +1481,16 @@ namespace CefSharp
return true;
}

auto frameId = StringUtils::ToClr(frame->GetIdentifier());

auto callbackFactory = browserAdapter->JavascriptCallbackFactory;

auto success = argList->GetBool(0);
auto callbackId = GetInt64(argList, 1);

auto pendingTask = name == kEvaluateJavascriptResponse ?
_pendingTaskRepository->RemovePendingTask(callbackId) :
_pendingTaskRepository->RemoveJavascriptCallbackPendingTask(callbackId);
_pendingTaskRepository->RemovePendingTask(frameId, callbackId) :
_pendingTaskRepository->RemoveJavascriptCallbackPendingTask(frameId, callbackId);

if (pendingTask != nullptr)
{
Expand Down
4 changes: 2 additions & 2 deletions CefSharp.Core.Runtime/Internals/ClientAdapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ namespace CefSharp

CloseAllPopups(true);

//this will dispose the repository and cancel all pending tasks
delete _pendingTaskRepository;
Copy link
Member

Choose a reason for hiding this comment

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

delete will call Dispose if it's implemented. Guess it's semantics, might be worth having pending task repository implement IDisposable.

_pendingTaskRepository->CancelPendingTasks();

_browser = nullptr;
_browserControl = nullptr;
Expand All @@ -80,6 +79,7 @@ namespace CefSharp
_tooltip = nullptr;
_browserAdapter = nullptr;
_popupBrowsers = nullptr;
_pendingTaskRepository = nullptr;
}

HWND GetBrowserHwnd() { return _browserHwnd; }
Expand Down
31 changes: 16 additions & 15 deletions CefSharp.Core.Runtime/Internals/JavascriptCallbackProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,27 @@ namespace CefSharp
}

auto browserWrapper = static_cast<CefBrowserWrapper^>(browser);
auto javascriptNameConverter = GetJavascriptNameConverter();

auto doneCallback = _pendingTasks->CreateJavascriptCallbackPendingTask(_callback->Id, timeout);

auto callbackMessage = CefProcessMessage::Create(kJavascriptCallbackRequest);
auto argList = callbackMessage->GetArgumentList();
SetInt64(argList, 0, doneCallback.Key);
SetInt64(argList, 1, _callback->Id);
auto paramList = CefListValue::Create();
for (int i = 0; i < parameters->Length; i++)
{
auto param = parameters[i];
SerializeV8Object(paramList, i, param, javascriptNameConverter);
}
argList->SetList(2, paramList);

auto frame = browserWrapper->Browser->GetFrameByIdentifier(StringUtils::ToNative(_callback->FrameId));

if (frame.get() && frame->IsValid())
{
auto javascriptNameConverter = GetJavascriptNameConverter();

auto doneCallback = _pendingTasks->CreateJavascriptCallbackPendingTask(_callback->FrameId, _callback->Id, timeout);

auto callbackMessage = CefProcessMessage::Create(kJavascriptCallbackRequest);
auto argList = callbackMessage->GetArgumentList();
SetInt64(argList, 0, doneCallback.Key);
SetInt64(argList, 1, _callback->Id);
auto paramList = CefListValue::Create();
for (int i = 0; i < parameters->Length; i++)
{
auto param = parameters[i];
SerializeV8Object(paramList, i, param, javascriptNameConverter);
}
argList->SetList(2, paramList);

frame->SendProcessMessage(CefProcessId::PID_RENDERER, callbackMessage);

return doneCallback.Value->Task;
Expand Down
49 changes: 48 additions & 1 deletion CefSharp.Test/Javascript/EvaluateScriptAsyncTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Text;
using System.Threading.Tasks;
using Bogus;
using CefSharp.Example;
using Xunit;
using Xunit.Abstractions;
using Xunit.Repeat;
Expand All @@ -26,6 +27,52 @@ public EvaluateScriptAsyncTests(ITestOutputHelper output, CefSharpFixture collec
this.collectionFixture = collectionFixture;
}

[Fact]
public async Task ShouldCancelAfterV8ContextChange()
{
Task evaluateCancelAfterDisposeTask;
using (var browser = new CefSharp.OffScreen.ChromiumWebBrowser(automaticallyCreateBrowser: false))
{
await browser.CreateBrowserAsync();

// no V8 context
var withoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("1+1"));
Assert.StartsWith("Unable to execute javascript at this time", withoutV8ContextException.Message);

Task<JavascriptResponse> evaluateWithoutV8ContextTask;
using (var frame = browser.GetMainFrame())
{
evaluateWithoutV8ContextTask = frame.EvaluateScriptAsync("1+2");
}

// V8 context
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);
var evaluateWithoutV8ContextResponse = await evaluateWithoutV8ContextTask;
Assert.True(evaluateWithoutV8ContextResponse.Success);
Assert.Equal(3, evaluateWithoutV8ContextResponse.Result);

var evaluateCancelAfterV8ContextChangeTask = browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");

// change V8 context
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);

await Assert.ThrowsAsync<TaskCanceledException>(() => evaluateCancelAfterV8ContextChangeTask);

evaluateCancelAfterDisposeTask = browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");
}
await Assert.ThrowsAsync<TaskCanceledException>(() => evaluateCancelAfterDisposeTask);
}

[Fact]
public async Task ShouldCancelOnCrash()
{
AssertInitialLoadComplete();

var task = Browser.EvaluateScriptAsync("new Promise(resolve => setTimeout(resolve, 1000))");
await Browser.LoadUrlAsync("chrome://crash");
await Assert.ThrowsAsync<TaskCanceledException>(() => task);
}

[Theory]
[InlineData(double.MaxValue, "Number.MAX_VALUE")]
[InlineData(double.MaxValue / 2, "Number.MAX_VALUE / 2")]
Expand Down Expand Up @@ -264,7 +311,7 @@ public async Task CanEvaluateScriptAsyncReturnArrayBuffer(int iteration)

var randomizer = new Randomizer();

var expected = randomizer.Utf16String(minLength: iteration, maxLength:iteration);
var expected = randomizer.Utf16String(minLength: iteration, maxLength: iteration);
var expectedBytes = Encoding.UTF8.GetBytes(expected);

var javascriptResponse = await Browser.EvaluateScriptAsync($"new TextEncoder().encode('{expected}').buffer");
Expand Down
87 changes: 87 additions & 0 deletions CefSharp.Test/Javascript/JavascriptCallbackTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Dynamic;
using System.Globalization;
using System.Threading.Tasks;
using CefSharp.Example;
using Xunit;
using Xunit.Abstractions;

Expand All @@ -23,6 +24,74 @@ public JavascriptCallbackTests(ITestOutputHelper output, CefSharpFixture collect
this.collectionFixture = collectionFixture;
}

[Fact]
public async Task ShouldCancelAfterV8ContextChange()
{
IJavascriptCallback callbackExecuteCancelAfterDisposeCallback;
Task callbackExecuteCancelAfterDisposeTask;
using (var browser = new CefSharp.OffScreen.ChromiumWebBrowser(automaticallyCreateBrowser: false))
{
await browser.CreateBrowserAsync();

// no V8 context
var withoutV8ContextException = await Assert.ThrowsAsync<Exception>(() => browser.EvaluateScriptAsync("(function() { return 1+1; })"));
Assert.StartsWith("Unable to execute javascript at this time", withoutV8ContextException.Message);

Task<JavascriptResponse> callbackExecuteWithoutV8ContextTask;
using (var frame = browser.GetMainFrame())
{
callbackExecuteWithoutV8ContextTask = frame.EvaluateScriptAsync("(function() { return 1+2; })");
}

// V8 context
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);

var callbackExecuteWithoutV8ContextResponse = await callbackExecuteWithoutV8ContextTask;
Assert.True(callbackExecuteWithoutV8ContextResponse.Success);
var callbackExecuteWithoutV8ContextCallback = (IJavascriptCallback)callbackExecuteWithoutV8ContextResponse.Result;
var callbackExecuteWithoutV8ContextExecuteResponse = await callbackExecuteWithoutV8ContextCallback.ExecuteAsync();
Assert.True(callbackExecuteWithoutV8ContextExecuteResponse.Success);
Assert.Equal(3, callbackExecuteWithoutV8ContextExecuteResponse.Result);

var callbackExecuteCancelAfterV8ContextResponse = await browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
Assert.True(callbackExecuteCancelAfterV8ContextResponse.Success);
var callbackExecuteCancelAfterV8ContextCallback = (IJavascriptCallback)callbackExecuteCancelAfterV8ContextResponse.Result;
var callbackExecuteCancelAfterV8ContextTask = callbackExecuteCancelAfterV8ContextCallback.ExecuteAsync();

// change V8 context
await browser.LoadUrlAsync(CefExample.HelloWorldUrl);

await Assert.ThrowsAsync<TaskCanceledException>(() => callbackExecuteCancelAfterV8ContextTask);
var callbackExecuteCancelAfterV8ContextResult = await callbackExecuteCancelAfterV8ContextCallback.ExecuteAsync();
Assert.False(callbackExecuteCancelAfterV8ContextResult.Success);
Assert.StartsWith("Unable to find JavascriptCallback with Id " + callbackExecuteCancelAfterV8ContextCallback.Id, callbackExecuteCancelAfterV8ContextResult.Message);

var callbackExecuteCancelAfterDisposeResponse = await browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
Assert.True(callbackExecuteCancelAfterDisposeResponse.Success);
callbackExecuteCancelAfterDisposeCallback = (IJavascriptCallback)callbackExecuteCancelAfterDisposeResponse.Result;
callbackExecuteCancelAfterDisposeTask = callbackExecuteCancelAfterDisposeCallback.ExecuteAsync();
}
Assert.False(callbackExecuteCancelAfterDisposeCallback.CanExecute);
await Assert.ThrowsAsync<TaskCanceledException>(() => callbackExecuteCancelAfterDisposeTask);
await Assert.ThrowsAsync<InvalidOperationException>(() => callbackExecuteCancelAfterDisposeCallback.ExecuteAsync());
}

[Fact]
public async Task ShouldCancelOnCrash()
{
AssertInitialLoadComplete();

var javascriptResponse = await Browser.EvaluateScriptAsync("(function() { return new Promise(resolve => setTimeout(resolve, 1000)); })");
Assert.True(javascriptResponse.Success);

var callback = (IJavascriptCallback)javascriptResponse.Result;

var task = callback.ExecuteAsync();

await Browser.LoadUrlAsync("chrome://crash");
await Assert.ThrowsAsync<TaskCanceledException>(() => task);
}

[Theory]
[InlineData("(function() { return Promise.resolve(53)})", 53)]
[InlineData("(function() { return Promise.resolve('53')})", "53")]
Expand Down Expand Up @@ -233,5 +302,23 @@ public async Task ShouldWorkWithExpandoObject()

output.WriteLine("Expected {0} : Actual {1}", expectedDateTime, actualDateTime);
}

[Fact]
public async Task ShouldWorkWhenExecutedMultipleTimes()
{
AssertInitialLoadComplete();

var javascriptResponse = await Browser.EvaluateScriptAsync("(function() { return 42; })");
Assert.True(javascriptResponse.Success);

var callback = (IJavascriptCallback)javascriptResponse.Result;

for (var i = 0; i < 3; i++)
{
var callbackResponse = await callback.ExecuteAsync();
Assert.True(callbackResponse.Success);
Assert.Equal(42, callbackResponse.Result);
}
}
}
}
33 changes: 33 additions & 0 deletions CefSharp/Internals/FramePendingTaskRepository.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright © 2015 The CefSharp Authors. All rights reserved.
//
// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file.

using System;
using System.Collections.Concurrent;
using System.Threading.Tasks;

namespace CefSharp.Internals
{
internal sealed class FramePendingTaskRepository<TResult> : IDisposable
{
public ConcurrentDictionary<long, TaskCompletionSource<TResult>> PendingTasks { get; } =
new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();
public ConcurrentDictionary<long, TaskCompletionSource<TResult>> CallbackPendingTasks { get; } =
new ConcurrentDictionary<long, TaskCompletionSource<TResult>>();

public void Dispose()
{
foreach (var tcs in PendingTasks.Values)
{
tcs.TrySetCanceled();
}
PendingTasks.Clear();

foreach (var tcs in CallbackPendingTasks.Values)
{
tcs.TrySetCanceled();
}
CallbackPendingTasks.Clear();
}
}
}
Loading