From 5f4e3b9d3c9fc62957d0bda9a0a7d3ae4830e1e9 Mon Sep 17 00:00:00 2001 From: liaosirui Date: Mon, 12 Oct 2020 10:52:44 +0800 Subject: [PATCH 1/2] Check if colorBuffer is in detroy list before using and resume such colorBuffer. delete print and comment --- src/anbox/graphics/emugl/Renderer.cpp | 26 ++++++++++++++++++++++---- src/anbox/graphics/emugl/Renderer.h | 3 ++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/anbox/graphics/emugl/Renderer.cpp b/src/anbox/graphics/emugl/Renderer.cpp index ab69187..74601fe 100644 --- a/src/anbox/graphics/emugl/Renderer.cpp +++ b/src/anbox/graphics/emugl/Renderer.cpp @@ -101,6 +101,18 @@ void Renderer::saveColorBuffer(ColorBufferRef* cbRef) { cbRef->closedTs = 0; } +void Renderer::resumeColorBuffer(ColorBufferRef* cbRef) { + RenderThreadInfo *tInfo = RenderThreadInfo::get(); + if (eraseDelayedCloseColorBufferLocked(cbRef->cb->getHndl(), cbRef->closedTs)) { + cbRef->refcount++; + cbRef->closedTs = 0; + int tid = tInfo->m_tid; + if (tid > 0) { + m_procOwnedColorBuffers[tid].insert(cbRef->cb->getHndl()); + } + } +} + HandleType Renderer::s_nextHandle = 0; void Renderer::finalize() { @@ -565,7 +577,7 @@ int Renderer::openColorBuffer(HandleType p_colorbuffer) { return 0; } -void Renderer::eraseDelayedCloseColorBufferLocked(HandleType cb, int64_t ts) { +bool Renderer::eraseDelayedCloseColorBufferLocked(HandleType cb, int64_t ts) { // Find the first delayed buffer with a timestamp <= |ts| auto it = std::lower_bound( m_colorBufferDelayedCloseList.begin(), @@ -578,10 +590,11 @@ void Renderer::eraseDelayedCloseColorBufferLocked(HandleType cb, int64_t ts) { // if this is the one we need - clear it out. if (it->cbHandle == cb) { it->cbHandle = 0; - break; + return true; } ++it; } + return false; } void Renderer::performDelayedColorBufferCloseLocked() { @@ -589,8 +602,8 @@ void Renderer::performDelayedColorBufferCloseLocked() { // timestamp change (end of previous second -> beginning of a next one), // but not for long - this is a workaround for race conditions, and they // are quick. - static constexpr int64_t kColorBufferClosingDelayMS = 2000; - + static constexpr int64_t kColorBufferClosingDelayMS = 10000; + const auto now = getCurrentLocalTimeStamp(); auto it = m_colorBufferDelayedCloseList.begin(); while (it != m_colorBufferDelayedCloseList.end() && @@ -718,6 +731,7 @@ bool Renderer::setWindowSurfaceColorBuffer(HandleType p_surface, } ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); + resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { DEBUG("%s: bad color buffer handle %#x", __FUNCTION__, p_colorbuffer); // bad colorbuffer handle @@ -740,6 +754,7 @@ void Renderer::readColorBuffer(HandleType p_colorbuffer, int x, int y, std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); + resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return; @@ -754,6 +769,7 @@ bool Renderer::updateColorBuffer(HandleType p_colorbuffer, int x, int y, std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); + resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; @@ -768,6 +784,7 @@ bool Renderer::bindColorBufferToTexture(HandleType p_colorbuffer) { std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); + resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; @@ -780,6 +797,7 @@ bool Renderer::bindColorBufferToRenderbuffer(HandleType p_colorbuffer) { std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); + resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; diff --git a/src/anbox/graphics/emugl/Renderer.h b/src/anbox/graphics/emugl/Renderer.h index db161b3..4acbe99 100644 --- a/src/anbox/graphics/emugl/Renderer.h +++ b/src/anbox/graphics/emugl/Renderer.h @@ -89,8 +89,9 @@ class Renderer : public anbox::graphics::Renderer { virtual ~Renderer(); void saveColorBuffer(ColorBufferRef* cbRef); + void resumeColorBuffer(ColorBufferRef* cbRef); - void eraseDelayedCloseColorBufferLocked(HandleType cb, int64_t ts); + bool eraseDelayedCloseColorBufferLocked(HandleType cb, int64_t ts); void performDelayedColorBufferCloseLocked(); -- Gitee From 606919ddd7b6363b170ea3462e7ee0b6ca3c9eb3 Mon Sep 17 00:00:00 2001 From: liaosirui Date: Tue, 13 Oct 2020 09:19:12 +0800 Subject: [PATCH 2/2] Fix bug that illegal iterator might be used. Use closedTs to help save buffer. --- src/anbox/graphics/emugl/Renderer.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/anbox/graphics/emugl/Renderer.cpp b/src/anbox/graphics/emugl/Renderer.cpp index 74601fe..0078b0b 100644 --- a/src/anbox/graphics/emugl/Renderer.cpp +++ b/src/anbox/graphics/emugl/Renderer.cpp @@ -103,7 +103,7 @@ void Renderer::saveColorBuffer(ColorBufferRef* cbRef) { void Renderer::resumeColorBuffer(ColorBufferRef* cbRef) { RenderThreadInfo *tInfo = RenderThreadInfo::get(); - if (eraseDelayedCloseColorBufferLocked(cbRef->cb->getHndl(), cbRef->closedTs)) { + if (cbRef->closedTs != 0 && eraseDelayedCloseColorBufferLocked(cbRef->cb->getHndl(), cbRef->closedTs)) { cbRef->refcount++; cbRef->closedTs = 0; int tid = tInfo->m_tid; @@ -731,13 +731,12 @@ bool Renderer::setWindowSurfaceColorBuffer(HandleType p_surface, } ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); - resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { DEBUG("%s: bad color buffer handle %#x", __FUNCTION__, p_colorbuffer); // bad colorbuffer handle return false; } - + resumeColorBuffer(&((*c).second)); (*w).second.first->setColorBuffer((*c).second.cb); saveColorBuffer(&c->second); if (w->second.second) { @@ -754,12 +753,11 @@ void Renderer::readColorBuffer(HandleType p_colorbuffer, int x, int y, std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); - resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return; } - + resumeColorBuffer(&((*c).second)); (*c).second.cb->readPixels(x, y, width, height, format, type, pixels); } @@ -769,12 +767,11 @@ bool Renderer::updateColorBuffer(HandleType p_colorbuffer, int x, int y, std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); - resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; } - + resumeColorBuffer(&((*c).second)); (*c).second.cb->subUpdate(x, y, width, height, format, type, pixels); return true; @@ -784,12 +781,11 @@ bool Renderer::bindColorBufferToTexture(HandleType p_colorbuffer) { std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); - resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; } - + resumeColorBuffer(&((*c).second)); return (*c).second.cb->bindToTexture(); } @@ -797,12 +793,11 @@ bool Renderer::bindColorBufferToRenderbuffer(HandleType p_colorbuffer) { std::unique_lock l(m_lock); ColorBufferMap::iterator c(m_colorbuffers.find(p_colorbuffer)); - resumeColorBuffer(&((*c).second)); if (c == m_colorbuffers.end()) { // bad colorbuffer handle return false; } - + resumeColorBuffer(&((*c).second)); return (*c).second.cb->bindToRenderbuffer(); } -- Gitee