From cbcaa107ebb72ce573a0f55c79b3a79bda72ce15 Mon Sep 17 00:00:00 2001 From: Matthew Waters Date: Tue, 18 Oct 2016 00:28:14 +1100 Subject: [PATCH] cfileio: fix leaks by not closing files in the destructor Numerous importers rely on the default C++ IOSystem implementation having the destructor close the file for them. The C IOSystem wrapper wasn't and instead assumed that the Close() method was going to be called. This brings the C IOSystem wrapper in line with the default C++ IOSystem by having the destructor close the file. --- code/CInterfaceIOWrapper.cpp | 134 +++++++++++++++++++++++++++++++++++ code/CInterfaceIOWrapper.h | 99 +++++--------------------- code/CMakeLists.txt | 1 + 3 files changed, 153 insertions(+), 81 deletions(-) create mode 100644 code/CInterfaceIOWrapper.cpp diff --git a/code/CInterfaceIOWrapper.cpp b/code/CInterfaceIOWrapper.cpp new file mode 100644 index 000000000..0a80c1990 --- /dev/null +++ b/code/CInterfaceIOWrapper.cpp @@ -0,0 +1,134 @@ +/* +--------------------------------------------------------------------------- +Open Asset Import Library (assimp) +--------------------------------------------------------------------------- + +Copyright (c) 2006-2016, assimp team + +All rights reserved. + +Redistribution and use of this software in source and binary forms, +with or without modification, are permitted provided that the following +conditions are met: + +* Redistributions of source code must retain the above + copyright notice, this list of conditions and the + following disclaimer. + +* Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the + following disclaimer in the documentation and/or other + materials provided with the distribution. + +* Neither the name of the assimp team, nor the names of its + contributors may be used to endorse or promote products + derived from this software without specific prior + written permission of the assimp team. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +--------------------------------------------------------------------------- +*/ + +/** @file aiFileIO -> IOSystem wrapper*/ + +#include "CInterfaceIOWrapper.h" + +namespace Assimp { + +CIOStreamWrapper::~CIOStreamWrapper(void) +{ + /* Various places depend on this destructor to close the file */ + if (mFile) { + mIO->mFileSystem->CloseProc(mIO->mFileSystem, mFile); + mFile = nullptr; + } +} + +// ................................................................... +size_t CIOStreamWrapper::Read(void* pvBuffer, + size_t pSize, + size_t pCount +){ + // need to typecast here as C has no void* + return mFile->ReadProc(mFile,(char*)pvBuffer,pSize,pCount); +} + +// ................................................................... +size_t CIOStreamWrapper::Write(const void* pvBuffer, + size_t pSize, + size_t pCount +){ + // need to typecast here as C has no void* + return mFile->WriteProc(mFile,(const char*)pvBuffer,pSize,pCount); +} + +// ................................................................... +aiReturn CIOStreamWrapper::Seek(size_t pOffset, + aiOrigin pOrigin +){ + return mFile->SeekProc(mFile,pOffset,pOrigin); +} + +// ................................................................... +size_t CIOStreamWrapper::Tell(void) const { + return mFile->TellProc(mFile); +} + +// ................................................................... +size_t CIOStreamWrapper::FileSize() const { + return mFile->FileSizeProc(mFile); +} + +// ................................................................... +void CIOStreamWrapper::Flush () { + return mFile->FlushProc(mFile); +} + +// ------------------------------------------------------------------------------------------------ +// Custom IOStream implementation for the C-API +bool CIOSystemWrapper::Exists( const char* pFile) const { + aiFile* p = mFileSystem->OpenProc(mFileSystem,pFile,"rb"); + if (p){ + mFileSystem->CloseProc(mFileSystem,p); + return true; + } + return false; +} + +// ................................................................... +char CIOSystemWrapper::getOsSeparator() const { +#ifndef _WIN32 + return '/'; +#else + return '\\'; +#endif +} + +// ................................................................... +IOStream* CIOSystemWrapper::Open(const char* pFile,const char* pMode) { + aiFile* p = mFileSystem->OpenProc(mFileSystem,pFile,pMode); + if (!p) { + return NULL; + } + return new CIOStreamWrapper(p, this); +} + +// ................................................................... +void CIOSystemWrapper::Close( IOStream* pFile) { + if (!pFile) { + return; + } + delete pFile; +} + +} diff --git a/code/CInterfaceIOWrapper.h b/code/CInterfaceIOWrapper.h index beffd8678..0172fb49f 100644 --- a/code/CInterfaceIOWrapper.h +++ b/code/CInterfaceIOWrapper.h @@ -50,106 +50,43 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. namespace Assimp { +class CIOSystemWrapper; + // ------------------------------------------------------------------------------------------------ // Custom IOStream implementation for the C-API class CIOStreamWrapper : public IOStream { - friend class CIOSystemWrapper; public: - - explicit CIOStreamWrapper(aiFile* pFile) - : mFile(pFile) + explicit CIOStreamWrapper(aiFile* pFile, CIOSystemWrapper* io) + : mFile(pFile), + mIO(io) {} + ~CIOStreamWrapper(void); - // ................................................................... - size_t Read(void* pvBuffer, - size_t pSize, - size_t pCount - ){ - // need to typecast here as C has no void* - return mFile->ReadProc(mFile,(char*)pvBuffer,pSize,pCount); - } - - // ................................................................... - size_t Write(const void* pvBuffer, - size_t pSize, - size_t pCount - ){ - // need to typecast here as C has no void* - return mFile->WriteProc(mFile,(const char*)pvBuffer,pSize,pCount); - } - - // ................................................................... - aiReturn Seek(size_t pOffset, - aiOrigin pOrigin - ){ - return mFile->SeekProc(mFile,pOffset,pOrigin); - } - - // ................................................................... - size_t Tell(void) const { - return mFile->TellProc(mFile); - } - - // ................................................................... - size_t FileSize() const { - return mFile->FileSizeProc(mFile); - } - - // ................................................................... - void Flush () { - return mFile->FlushProc(mFile); - } + size_t Read(void* pvBuffer, size_t pSize, size_t pCount); + size_t Write(const void* pvBuffer, size_t pSize, size_t pCount); + aiReturn Seek(size_t pOffset, aiOrigin pOrigin); + size_t Tell(void) const; + size_t FileSize() const; + void Flush(); private: aiFile* mFile; + CIOSystemWrapper* mIO; }; -// ------------------------------------------------------------------------------------------------ -// Custom IOStream implementation for the C-API class CIOSystemWrapper : public IOSystem { + friend class CIOStreamWrapper; public: explicit CIOSystemWrapper(aiFileIO* pFile) : mFileSystem(pFile) {} - // ................................................................... - bool Exists( const char* pFile) const { - aiFile* p = mFileSystem->OpenProc(mFileSystem,pFile,"rb"); - if (p){ - mFileSystem->CloseProc(mFileSystem,p); - return true; - } - return false; - } - - // ................................................................... - char getOsSeparator() const { -#ifndef _WIN32 - return '/'; -#else - return '\\'; -#endif - } - - // ................................................................... - IOStream* Open(const char* pFile,const char* pMode = "rb") { - aiFile* p = mFileSystem->OpenProc(mFileSystem,pFile,pMode); - if (!p) { - return NULL; - } - return new CIOStreamWrapper(p); - } - - // ................................................................... - void Close( IOStream* pFile) { - if (!pFile) { - return; - } - mFileSystem->CloseProc(mFileSystem,((CIOStreamWrapper*) pFile)->mFile); - delete pFile; - } + bool Exists( const char* pFile) const; + char getOsSeparator() const; + IOStream* Open(const char* pFile,const char* pMode = "rb"); + void Close( IOStream* pFile); private: aiFileIO* mFileSystem; }; diff --git a/code/CMakeLists.txt b/code/CMakeLists.txt index 10979df5c..771b9e24c 100644 --- a/code/CMakeLists.txt +++ b/code/CMakeLists.txt @@ -128,6 +128,7 @@ SET( Common_SRCS DefaultIOStream.h DefaultIOSystem.cpp DefaultIOSystem.h + CInterfaceIOWrapper.cpp CInterfaceIOWrapper.h Hash.h Importer.cpp