HostConnection: Use unique_ptr in HostConnection class members.
Fuchsia LeakSanitizer has found a memory leak in
goldfish ICD due to not deleting "m_vkEnc" in dtor
of HostConnection. Using smart pointers instead of
"new" and "delete" will help us prevent bugs like
this.
This change removes most new/delete usages within
HostConnection, and use std::unique_ptr instead.
Bug: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=46839
TEST=fuchsia escher_tests, scenic_tests with LSan turned on.
Change-Id: I42b46ee4a25c872e7eb493bf55228f9e017b5887
diff --git a/system/OpenglSystemCommon/HostConnection.cpp b/system/OpenglSystemCommon/HostConnection.cpp
index f308475..a1b17b1 100644
--- a/system/OpenglSystemCommon/HostConnection.cpp
+++ b/system/OpenglSystemCommon/HostConnection.cpp
@@ -352,11 +352,6 @@
static GoldfishProcessPipe m_goldfishProcessPipe;
HostConnection::HostConnection() :
- m_stream(NULL),
- m_glEnc(NULL),
- m_gl2Enc(NULL),
- m_vkEnc(NULL),
- m_rcEnc(NULL),
m_checksumHelper(),
m_glExtensions(),
m_grallocOnly(true),
@@ -369,15 +364,11 @@
// round-trip to ensure that queued commands have been processed
// before process pipe closure is detected.
if (m_rcEnc) {
- (void) m_rcEnc->rcGetRendererVersion(m_rcEnc);
+ (void)m_rcEnc->rcGetRendererVersion(m_rcEnc.get());
}
if (m_grallocType == GRALLOC_TYPE_MINIGBM) {
delete m_grallocHelper;
}
- delete m_stream;
- delete m_glEnc;
- delete m_gl2Enc;
- delete m_rcEnc;
if (m_rendernodeFdOwned) {
close(m_rendernodeFd);
@@ -393,7 +384,8 @@
switch (connType) {
case HOST_CONNECTION_ADDRESS_SPACE: {
- AddressSpaceStream *stream = createAddressSpaceStream(STREAM_BUFFER_SIZE);
+ auto stream = std::unique_ptr<AddressSpaceStream>(
+ createAddressSpaceStream(STREAM_BUFFER_SIZE));
if (!stream) {
ALOGE("Failed to create AddressSpaceStream for host connection!!!\n");
delete con;
@@ -401,13 +393,13 @@
}
con->m_connectionType = HOST_CONNECTION_ADDRESS_SPACE;
con->m_grallocType = GRALLOC_TYPE_RANCHU;
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_grallocHelper = &m_goldfishGralloc;
con->m_processPipe = &m_goldfishProcessPipe;
break;
}
case HOST_CONNECTION_QEMU_PIPE: {
- QemuPipeStream *stream = new QemuPipeStream(STREAM_BUFFER_SIZE);
+ auto stream = std::make_unique<QemuPipeStream>(STREAM_BUFFER_SIZE);
if (!stream) {
ALOGE("Failed to create QemuPipeStream for host connection!!!\n");
delete con;
@@ -415,13 +407,12 @@
}
if (stream->connect() < 0) {
ALOGE("Failed to connect to host (QemuPipeStream)!!!\n");
- delete stream;
delete con;
return NULL;
}
con->m_connectionType = HOST_CONNECTION_QEMU_PIPE;
con->m_grallocType = GRALLOC_TYPE_RANCHU;
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_grallocHelper = &m_goldfishGralloc;
con->m_processPipe = &m_goldfishProcessPipe;
break;
@@ -433,7 +424,7 @@
return NULL;
break;
#else
- TcpStream *stream = new TcpStream(STREAM_BUFFER_SIZE);
+ auto stream = std::make_unique<TcpStream>(STREAM_BUFFER_SIZE);
if (!stream) {
ALOGE("Failed to create TcpStream for host connection!!!\n");
delete con;
@@ -442,13 +433,12 @@
if (stream->connect("10.0.2.2", STREAM_PORT_NUM) < 0) {
ALOGE("Failed to connect to host (TcpStream)!!!\n");
- delete stream;
delete con;
return NULL;
}
con->m_connectionType = HOST_CONNECTION_TCP;
con->m_grallocType = GRALLOC_TYPE_RANCHU;
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_grallocHelper = &m_goldfishGralloc;
con->m_processPipe = &m_goldfishProcessPipe;
break;
@@ -456,7 +446,7 @@
}
#ifdef VIRTIO_GPU
case HOST_CONNECTION_VIRTIO_GPU: {
- VirtioGpuStream *stream = new VirtioGpuStream(STREAM_BUFFER_SIZE);
+ auto stream = std::make_unique<VirtioGpuStream>(STREAM_BUFFER_SIZE);
if (!stream) {
ALOGE("Failed to create VirtioGpu for host connection!!!\n");
delete con;
@@ -464,13 +454,12 @@
}
if (stream->connect() < 0) {
ALOGE("Failed to connect to host (VirtioGpu)!!!\n");
- delete stream;
delete con;
return NULL;
}
con->m_connectionType = HOST_CONNECTION_VIRTIO_GPU;
con->m_grallocType = GRALLOC_TYPE_MINIGBM;
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_rendernodeFdOwned = false;
con->m_rendernodeFdOwned = stream->getRendernodeFd();
MinigbmGralloc* m = new MinigbmGralloc;
@@ -480,7 +469,8 @@
break;
}
case HOST_CONNECTION_VIRTIO_GPU_PIPE: {
- VirtioGpuPipeStream *stream = new VirtioGpuPipeStream(STREAM_BUFFER_SIZE);
+ auto stream =
+ std::make_unique<VirtioGpuPipeStream>(STREAM_BUFFER_SIZE);
if (!stream) {
ALOGE("Failed to create VirtioGpu for host connection!!!\n");
delete con;
@@ -488,13 +478,12 @@
}
if (stream->connect() < 0) {
ALOGE("Failed to connect to host (VirtioGpu)!!!\n");
- delete stream;
delete con;
return NULL;
}
con->m_connectionType = HOST_CONNECTION_VIRTIO_GPU_PIPE;
con->m_grallocType = getGrallocTypeFromProperty();
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_rendernodeFdOwned = false;
con->m_rendernodeFd = stream->getRendernodeFd();
switch (con->m_grallocType) {
@@ -516,7 +505,8 @@
}
#ifndef HOST_BUILD
case HOST_CONNECTION_VIRTIO_GPU_ADDRESS_SPACE: {
- AddressSpaceStream *stream = createVirtioGpuAddressSpaceStream(STREAM_BUFFER_SIZE);
+ auto stream = std::unique_ptr<AddressSpaceStream>(
+ createVirtioGpuAddressSpaceStream(STREAM_BUFFER_SIZE));
if (!stream) {
ALOGE("Failed to create virtgpu AddressSpaceStream for host connection!!!\n");
delete con;
@@ -524,7 +514,7 @@
}
con->m_connectionType = HOST_CONNECTION_VIRTIO_GPU_ADDRESS_SPACE;
con->m_grallocType = getGrallocTypeFromProperty();
- con->m_stream = stream;
+ con->m_stream = std::move(stream);
con->m_rendernodeFdOwned = false;
con->m_rendernodeFd = stream->getRendernodeFd();
switch (con->m_grallocType) {
@@ -610,18 +600,19 @@
GLEncoder *HostConnection::glEncoder()
{
if (!m_glEnc) {
- m_glEnc = new GLEncoder(m_stream, checksumHelper());
+ m_glEnc = std::make_unique<GLEncoder>(m_stream.get(), checksumHelper());
DBG("HostConnection::glEncoder new encoder %p, tid %d",
m_glEnc, getCurrentThreadId());
m_glEnc->setContextAccessor(s_getGLContext);
}
- return m_glEnc;
+ return m_glEnc.get();
}
GL2Encoder *HostConnection::gl2Encoder()
{
if (!m_gl2Enc) {
- m_gl2Enc = new GL2Encoder(m_stream, checksumHelper());
+ m_gl2Enc =
+ std::make_unique<GL2Encoder>(m_stream.get(), checksumHelper());
DBG("HostConnection::gl2Encoder new encoder %p, tid %d",
m_gl2Enc, getCurrentThreadId());
m_gl2Enc->setContextAccessor(s_getGL2Context);
@@ -630,45 +621,48 @@
getDrawCallFlushIntervalFromProperty());
m_gl2Enc->setHasAsyncUnmapBuffer(m_rcEnc->hasAsyncUnmapBuffer());
}
- return m_gl2Enc;
+ return m_gl2Enc.get();
}
VkEncoder *HostConnection::vkEncoder()
{
if (!m_vkEnc) {
- m_vkEnc = new VkEncoder(m_stream);
+ m_vkEnc = std::make_unique<VkEncoder>(m_stream.get());
}
- return m_vkEnc;
+ return m_vkEnc.get();
}
ExtendedRCEncoderContext *HostConnection::rcEncoder()
{
if (!m_rcEnc) {
- m_rcEnc = new ExtendedRCEncoderContext(m_stream, checksumHelper());
- setChecksumHelper(m_rcEnc);
- queryAndSetSyncImpl(m_rcEnc);
- queryAndSetDmaImpl(m_rcEnc);
- queryAndSetGLESMaxVersion(m_rcEnc);
- queryAndSetNoErrorState(m_rcEnc);
- queryAndSetHostCompositionImpl(m_rcEnc);
- queryAndSetDirectMemSupport(m_rcEnc);
- queryAndSetVulkanSupport(m_rcEnc);
- queryAndSetDeferredVulkanCommandsSupport(m_rcEnc);
- queryAndSetVulkanNullOptionalStringsSupport(m_rcEnc);
- queryAndSetVulkanCreateResourcesWithRequirementsSupport(m_rcEnc);
- queryAndSetVulkanIgnoredHandles(m_rcEnc);
- queryAndSetYUVCache(m_rcEnc);
- queryAndSetAsyncUnmapBuffer(m_rcEnc);
- queryAndSetVirtioGpuNext(m_rcEnc);
- queryHasSharedSlotsHostMemoryAllocator(m_rcEnc);
- queryAndSetVulkanFreeMemorySync(m_rcEnc);
- queryAndSetVirtioGpuNativeSync(m_rcEnc);
- queryAndSetVulkanShaderFloat16Int8Support(m_rcEnc);
+ m_rcEnc = std::make_unique<ExtendedRCEncoderContext>(m_stream.get(),
+ checksumHelper());
+
+ ExtendedRCEncoderContext* rcEnc = m_rcEnc.get();
+ setChecksumHelper(rcEnc);
+ queryAndSetSyncImpl(rcEnc);
+ queryAndSetDmaImpl(rcEnc);
+ queryAndSetGLESMaxVersion(rcEnc);
+ queryAndSetNoErrorState(rcEnc);
+ queryAndSetHostCompositionImpl(rcEnc);
+ queryAndSetDirectMemSupport(rcEnc);
+ queryAndSetVulkanSupport(rcEnc);
+ queryAndSetDeferredVulkanCommandsSupport(rcEnc);
+ queryAndSetVulkanNullOptionalStringsSupport(rcEnc);
+ queryAndSetVulkanCreateResourcesWithRequirementsSupport(rcEnc);
+ queryAndSetVulkanIgnoredHandles(rcEnc);
+ queryAndSetYUVCache(rcEnc);
+ queryAndSetAsyncUnmapBuffer(rcEnc);
+ queryAndSetVirtioGpuNext(rcEnc);
+ queryHasSharedSlotsHostMemoryAllocator(rcEnc);
+ queryAndSetVulkanFreeMemorySync(rcEnc);
+ queryAndSetVirtioGpuNativeSync(rcEnc);
+ queryAndSetVulkanShaderFloat16Int8Support(rcEnc);
if (m_processPipe) {
- m_processPipe->processPipeInit(m_connectionType, m_rcEnc);
+ m_processPipe->processPipeInit(m_connectionType, rcEnc);
}
}
- return m_rcEnc;
+ return m_rcEnc.get();
}
int HostConnection::getOrCreateRendernodeFd() {
@@ -699,7 +693,7 @@
{
EGLThreadInfo *ti = getEGLThreadInfo();
if (ti->hostConn) {
- return ti->hostConn->m_glEnc;
+ return ti->hostConn->m_glEnc.get();
}
return NULL;
}
@@ -708,7 +702,7 @@
{
EGLThreadInfo *ti = getEGLThreadInfo();
if (ti->hostConn) {
- return ti->hostConn->m_gl2Enc;
+ return ti->hostConn->m_gl2Enc.get();
}
return NULL;
}
diff --git a/system/OpenglSystemCommon/HostConnection.h b/system/OpenglSystemCommon/HostConnection.h
index 3b4772a..e297b37 100644
--- a/system/OpenglSystemCommon/HostConnection.h
+++ b/system/OpenglSystemCommon/HostConnection.h
@@ -30,6 +30,7 @@
#include <utils/threads.h>
#endif
+#include <memory>
#include <string>
class GLEncoder;
@@ -219,14 +220,16 @@
private:
HostConnectionType m_connectionType;
GrallocType m_grallocType;
- IOStream *m_stream;
- GLEncoder *m_glEnc;
- GL2Encoder *m_gl2Enc;
- goldfish_vk::VkEncoder *m_vkEnc;
- ExtendedRCEncoderContext *m_rcEnc;
+
+ std::unique_ptr<IOStream> m_stream;
+ std::unique_ptr<GLEncoder> m_glEnc;
+ std::unique_ptr<GL2Encoder> m_gl2Enc;
+ std::unique_ptr<goldfish_vk::VkEncoder> m_vkEnc;
+ std::unique_ptr<ExtendedRCEncoderContext> m_rcEnc;
+
ChecksumCalculator m_checksumHelper;
- Gralloc *m_grallocHelper;
- ProcessPipe *m_processPipe;
+ Gralloc* m_grallocHelper = nullptr;
+ ProcessPipe* m_processPipe = nullptr;
std::string m_glExtensions;
bool m_grallocOnly;
bool m_noHostError;
diff --git a/system/vulkan_enc/VkEncoder.cpp b/system/vulkan_enc/VkEncoder.cpp
index 9fdbe84..8a76c92 100644
--- a/system/vulkan_enc/VkEncoder.cpp
+++ b/system/vulkan_enc/VkEncoder.cpp
@@ -121,6 +121,8 @@
VkEncoder::VkEncoder(IOStream *stream) :
mImpl(new VkEncoder::Impl(stream)) { }
+VkEncoder::~VkEncoder() = default;
+
void VkEncoder::flush() {
mImpl->flush();
}
