diff --git a/distribution/changelog.txt b/distribution/changelog.txt index 9658d0464b..08763a2eb1 100644 --- a/distribution/changelog.txt +++ b/distribution/changelog.txt @@ -24,7 +24,6 @@ - Fix: [#22457] Potential crash opening the scenario select window. - Fix: [#22520] Virtual floor no longer appears when holding modifier keys during track construction. - Fix: [#22582] Lighting effects are not enabled/disabled correctly, making the game appear frozen. -- Fix: [#22593] Memory leak on Android builds when reading zip files. - Fix: [#22606] Virtual floor is sometimes drawn above the path when placing paths. - Fix: [#22625] Fix compilation with orignal ride ratings. diff --git a/src/openrct2/Context.cpp b/src/openrct2/Context.cpp index a3851c8bd4..6078d210fd 100644 --- a/src/openrct2/Context.cpp +++ b/src/openrct2/Context.cpp @@ -719,8 +719,8 @@ namespace OpenRCT2 { if (String::IEquals(Path::GetExtension(path), ".sea")) { - const auto data = DecryptSea(fs::u8path(path)); - auto ms = MemoryStream(data.data(), data.size()); + auto data = DecryptSea(fs::u8path(path)); + auto ms = MemoryStream(data.data(), data.size(), MEMORY_ACCESS::READ); if (!LoadParkFromStream(&ms, path, loadTitleScreenOnFail, asScenario)) { throw std::runtime_error(".sea file may have been renamed."); @@ -1060,14 +1060,14 @@ namespace OpenRCT2 { #ifndef DISABLE_HTTP // Download park and open it using its temporary filename - const auto data = DownloadPark(gOpenRCT2StartupActionPath); + auto data = DownloadPark(gOpenRCT2StartupActionPath); if (data.empty()) { nextScene = GetTitleScene(); break; } - auto ms = MemoryStream(data.data(), data.size()); + auto ms = MemoryStream(data.data(), data.size(), MEMORY_ACCESS::READ); if (!LoadParkFromStream(&ms, gOpenRCT2StartupActionPath, true)) { Console::Error::WriteLine("Failed to load '%s'", gOpenRCT2StartupActionPath); diff --git a/src/openrct2/core/MemoryStream.cpp b/src/openrct2/core/MemoryStream.cpp index 058c1ea9e9..a0da18d123 100644 --- a/src/openrct2/core/MemoryStream.cpp +++ b/src/openrct2/core/MemoryStream.cpp @@ -11,7 +11,6 @@ #include "Memory.hpp" -#include #include namespace OpenRCT2 @@ -21,53 +20,44 @@ namespace OpenRCT2 _access = copy._access; _dataCapacity = copy._dataCapacity; _dataSize = copy._dataSize; - _position = copy._position; if (_access & MEMORY_ACCESS::OWNER) { - // Create a copy when the memory stream owns the memory. - _data = Memory::Allocate(_dataCapacity); + _data = Memory::Allocate(_dataCapacity); std::memcpy(_data, copy._data, _dataCapacity); - } - else - { - // External pointer. - _data = copy._data; + _position = reinterpret_cast(reinterpret_cast(_data) + copy.GetPosition()); } } MemoryStream::MemoryStream(size_t capacity) { _dataCapacity = capacity; - _data = Memory::Allocate(capacity); - _position = 0; + _data = Memory::Allocate(capacity); + _position = _data; } - MemoryStream::MemoryStream(void* data, size_t dataSize) - : _access{ MEMORY_ACCESS::READ | MEMORY_ACCESS::WRITE } - , _dataCapacity{ dataSize } - , _dataSize{ dataSize } - , _data{ static_cast(data) } - , _position{ 0 } + MemoryStream::MemoryStream(void* data, size_t dataSize, uint8_t access) { + _access = access; + _dataCapacity = dataSize; + _dataSize = dataSize; + _data = data; + _position = _data; } MemoryStream::MemoryStream(const void* data, size_t dataSize) - : _access{ MEMORY_ACCESS::READ } - , _dataCapacity{ dataSize } - , _dataSize{ dataSize } - , _data{ static_cast(const_cast(data)) } - , _position{ 0 } + : MemoryStream(const_cast(data), dataSize, MEMORY_ACCESS::READ) { } - MemoryStream::MemoryStream(const void* data, size_t dataSize, bool) - : _access{ MEMORY_ACCESS::READ | MEMORY_ACCESS::OWNER } - , _dataCapacity{ dataSize } - , _dataSize{ dataSize } - , _data{ static_cast(const_cast(data)) } - , _position{ 0 } + MemoryStream::MemoryStream(std::vector&& v) { + _access = MEMORY_ACCESS::OWNER; + _dataCapacity = v.size(); + _dataSize = v.size(); + _data = Memory::Allocate(v.size()); + _position = _data; + std::memcpy(_data, v.data(), v.size()); } MemoryStream::MemoryStream(MemoryStream&& mv) noexcept @@ -78,7 +68,7 @@ namespace OpenRCT2 , _position(mv._position) { mv._data = nullptr; - mv._position = 0; + mv._position = nullptr; mv._dataCapacity = 0; mv._dataSize = 0; } @@ -98,11 +88,6 @@ namespace OpenRCT2 { if (this != &mv) { - if (_access & MEMORY_ACCESS::OWNER) - { - Memory::Free(_data); - } - _access = mv._access; _dataCapacity = mv._dataCapacity; _data = mv._data; @@ -110,7 +95,7 @@ namespace OpenRCT2 _position = mv._position; mv._data = nullptr; - mv._position = 0; + mv._position = nullptr; mv._dataCapacity = 0; mv._dataSize = 0; } @@ -120,7 +105,20 @@ namespace OpenRCT2 const void* MemoryStream::GetData() const { - return static_cast(_data); + return _data; + } + + void* MemoryStream::GetDataCopy() const + { + auto result = Memory::Allocate(_dataSize); + std::memcpy(result, _data, _dataSize); + return result; + } + + void* MemoryStream::TakeData() + { + _access &= ~MEMORY_ACCESS::OWNER; + return _data; } bool MemoryStream::CanRead() const @@ -140,7 +138,7 @@ namespace OpenRCT2 uint64_t MemoryStream::GetPosition() const { - return _position; + return static_cast(reinterpret_cast(_position) - reinterpret_cast(_data)); } void MemoryStream::SetPosition(uint64_t position) @@ -169,18 +167,19 @@ namespace OpenRCT2 { throw IOException("New position out of bounds."); } - _position = newPosition; + _position = reinterpret_cast(reinterpret_cast(_data) + static_cast(newPosition)); } void MemoryStream::Read(void* buffer, uint64_t length) { - if (_position + length > _dataSize) + uint64_t position = GetPosition(); + if (position + length > _dataSize) { throw IOException("Attempted to read past end of stream."); } - std::memcpy(buffer, _data + _position, length); - _position += length; + std::memcpy(buffer, _position, length); + _position = reinterpret_cast(reinterpret_cast(_position) + length); } void MemoryStream::Read1(void* buffer) @@ -218,12 +217,23 @@ namespace OpenRCT2 void MemoryStream::Write(const void* buffer, uint64_t length) { - const auto nextPosition = _position + length; - EnsureCapacity(nextPosition); + uint64_t position = GetPosition(); + uint64_t nextPosition = position + length; + if (nextPosition > _dataCapacity) + { + if (_access & MEMORY_ACCESS::OWNER) + { + EnsureCapacity(static_cast(nextPosition)); + } + else + { + throw IOException("Attempted to write past end of stream."); + } + } - std::memcpy(_data + _position, buffer, length); - _position += length; - _dataSize = std::max(_dataSize, nextPosition); + std::memcpy(_position, buffer, length); + _position = reinterpret_cast(reinterpret_cast(_position) + length); + _dataSize = std::max(_dataSize, static_cast(nextPosition)); } void MemoryStream::Write1(const void* buffer) @@ -254,25 +264,24 @@ namespace OpenRCT2 void MemoryStream::Clear() { _dataSize = 0; - _position = 0; + SetPosition(0); } void MemoryStream::EnsureCapacity(size_t capacity) { - if (capacity <= _dataCapacity) + if (_dataCapacity < capacity) { - return; + size_t newCapacity = std::max(8, _dataCapacity); + while (newCapacity < capacity) + { + newCapacity *= 2; + } + + uint64_t position = GetPosition(); + _dataCapacity = newCapacity; + _data = Memory::Reallocate(_data, _dataCapacity); + _position = reinterpret_cast(reinterpret_cast(_data) + static_cast(position)); } - - if ((_access & MEMORY_ACCESS::OWNER) == 0) - { - throw IOException("Can not grow external memory"); - } - - size_t newCapacity = std::max(32, std::bit_ceil(capacity)); - - _dataCapacity = newCapacity; - _data = Memory::Reallocate(_data, _dataCapacity); } } // namespace OpenRCT2 diff --git a/src/openrct2/core/MemoryStream.h b/src/openrct2/core/MemoryStream.h index e67233932a..33363a52fe 100644 --- a/src/openrct2/core/MemoryStream.h +++ b/src/openrct2/core/MemoryStream.h @@ -11,6 +11,8 @@ #include "IStream.hpp" +#include + namespace OpenRCT2 { namespace MEMORY_ACCESS @@ -29,24 +31,24 @@ namespace OpenRCT2 uint8_t _access = MEMORY_ACCESS::READ | MEMORY_ACCESS::WRITE | MEMORY_ACCESS::OWNER; size_t _dataCapacity = 0; size_t _dataSize = 0; - std::byte* _data = nullptr; - size_t _position = 0; + void* _data = nullptr; + void* _position = nullptr; public: MemoryStream() = default; MemoryStream(const MemoryStream& copy); MemoryStream(MemoryStream&& mv) noexcept; explicit MemoryStream(size_t capacity); - MemoryStream(void* data, size_t dataSize); + MemoryStream(void* data, size_t dataSize, uint8_t access = MEMORY_ACCESS::READ); MemoryStream(const void* data, size_t dataSize); - // Constructor that takes ownership, the memory has to be allocated by Memory::Allocate, this function only exists for - // Android interop reasons. - MemoryStream(const void* data, size_t dataSize, bool); + MemoryStream(std::vector&& v); virtual ~MemoryStream(); MemoryStream& operator=(MemoryStream&& mv) noexcept; const void* GetData() const override; + void* GetDataCopy() const; + void* TakeData(); /////////////////////////////////////////////////////////////////////////// // ISteam methods @@ -66,6 +68,18 @@ namespace OpenRCT2 void Read8(void* buffer) override; void Read16(void* buffer) override; + template void Read(void* buffer) + { + uint64_t position = GetPosition(); + if (position + N > _dataSize) + { + throw IOException("Attempted to read past end of stream."); + } + + std::memcpy(buffer, _position, N); + _position = reinterpret_cast(reinterpret_cast(_position) + N); + } + void Write(const void* buffer, uint64_t length) override; void Write1(const void* buffer) override; void Write2(const void* buffer) override; @@ -73,33 +87,33 @@ namespace OpenRCT2 void Write8(const void* buffer) override; void Write16(const void* buffer) override; + template void Write(const void* buffer) + { + uint64_t position = GetPosition(); + uint64_t nextPosition = position + N; + if (nextPosition > _dataCapacity) + { + if (_access & MEMORY_ACCESS::OWNER) + { + EnsureCapacity(static_cast(nextPosition)); + } + else + { + throw IOException("Attempted to write past end of stream."); + } + } + + std::memcpy(_position, buffer, N); + _position = reinterpret_cast(reinterpret_cast(_position) + N); + _dataSize = std::max(_dataSize, static_cast(nextPosition)); + } + uint64_t TryRead(void* buffer, uint64_t length) override; void Clear(); private: void EnsureCapacity(size_t capacity); - - template void Read(void* buffer) - { - if (_position + N > _dataSize) - { - throw IOException("Attempted to read past end of stream."); - } - - std::memcpy(buffer, _data + _position, N); - _position += N; - } - - template void Write(const void* buffer) - { - const auto nextPosition = _position + N; - EnsureCapacity(nextPosition); - - std::memcpy(_data + _position, buffer, N); - _position += N; - _dataSize = std::max(_dataSize, nextPosition); - } }; } // namespace OpenRCT2 diff --git a/src/openrct2/core/ZipAndroid.cpp b/src/openrct2/core/ZipAndroid.cpp index 2fb20a1818..cfd17ca736 100644 --- a/src/openrct2/core/ZipAndroid.cpp +++ b/src/openrct2/core/ZipAndroid.cpp @@ -111,31 +111,13 @@ public: auto dataPtr = reinterpret_cast(ptr); auto dataSize = this->GetFileSize(index); - auto res = std::vector(dataPtr, dataPtr + dataSize); - - Memory::Free(dataPtr); - return res; + return std::vector(dataPtr, dataPtr + dataSize); } std::unique_ptr GetFileStream(std::string_view path) const override { - // retrieve the JNI environment. - JNIEnv* env = (JNIEnv*)SDL_AndroidGetJNIEnv(); - - jclass zipClass = env->GetObjectClass(_zip); - jstring javaPath = env->NewStringUTF(path.data()); - jmethodID indexMethod = env->GetMethodID(zipClass, "getFileIndex", "(Ljava/lang/String;)I"); - jint index = env->CallIntMethod(_zip, indexMethod, javaPath); - - jmethodID fileMethod = env->GetMethodID(zipClass, "getFile", "(I)J"); - jlong ptr = env->CallLongMethod(_zip, fileMethod, index); - - auto dataPtr = reinterpret_cast(ptr); - auto dataSize = this->GetFileSize(index); - - // The Java code for getFile uses Java_io_openrct2_ZipArchive_allocBytes which - // allocates memory using Memory::Allocate so its safe for MemoryStream to take full ownershp. - return std::make_unique(dataPtr, dataSize, true); + auto data = GetFileData(path); + return std::make_unique(std::move(data)); } void SetFileData(std::string_view path, std::vector&& data) override