Commit 9457a610 authored by Vasiliy Telezhnikov's avatar Vasiliy Telezhnikov Committed by Commit Bot

Fix freeing DirectContextProvider on wrong thread

GLPixelBufferRGBAResult in GLRendererCopier hold reference to context
provider. When the results are sent to different thread we force all
GPU work to happen before in ReadPixelsWorkflow::Finish, but we still
hold the reference. If the results will outlive the
SkiaOutputSurfaceImplOnGpu it will be the last reference and
DirectContextProvider will be destroyed on a wrong thread.

This CL fixes this by dropping reference to context_provider when we
don't need it anymore.

Bug: 1035881
Change-Id: I6e0bfe7013a61f96ec7f33d40bae377d6831ffc9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041800Reviewed-by: default avatarJonathan Backer <backer@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739030}
parent 7a4c4be7
......@@ -439,6 +439,7 @@ class GLPixelBufferRGBAResult : public CopyOutputResult {
if (cached_bitmap()->readyToDraw())
return *cached_bitmap();
DCHECK(context_provider_);
SkBitmap result_bitmap;
// size() was clamped to render pass or framebuffer size. If we can't
// allocate it then OOM.
......@@ -454,12 +455,18 @@ class GLPixelBufferRGBAResult : public CopyOutputResult {
// anymore.
context_provider_->ContextGL()->DeleteBuffers(1, &transfer_buffer_);
transfer_buffer_ = 0;
// We don't need context provider anymore. If these CopyOutputResults will
// be sent to different thread we might end holding last reference to
// context provider, so drop it now.
context_provider_.reset();
return *cached_bitmap();
}
private:
const gfx::ColorSpace color_space_;
const scoped_refptr<ContextProvider> context_provider_;
mutable scoped_refptr<ContextProvider> context_provider_;
mutable GLuint transfer_buffer_;
const bool is_upside_down_;
const bool swap_red_and_blue_;
......
......@@ -121,6 +121,7 @@ DirectContextProvider::DirectContextProvider(
}
DirectContextProvider::~DirectContextProvider() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
if (decoder_)
Destroy();
}
......
......@@ -179,6 +179,8 @@ class VIZ_SERVICE_EXPORT DirectContextProvider
GLuint framebuffer_id_ = 0;
THREAD_CHECKER(thread_checker_);
base::ObserverList<ContextLostObserver>::Unchecked observers_;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment