From 8aa25f33ad21ab5cd68c49e38a24ffefed587ec2 Mon Sep 17 00:00:00 2001 From: Marc Gilleron Date: Mon, 4 Nov 2024 19:17:18 +0000 Subject: [PATCH] Fix SQLite connection leaks --- doc/source/changelog.md | 1 + streams/sqlite/voxel_stream_sqlite.cpp | 35 +++++++++++++++----------- streams/sqlite/voxel_stream_sqlite.h | 18 +++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) diff --git a/doc/source/changelog.md b/doc/source/changelog.md index 502405d47..fa424fd79 100644 --- a/doc/source/changelog.md +++ b/doc/source/changelog.md @@ -23,6 +23,7 @@ Primarily developped with Godot 4.3. - Fixed potential crash when when using the Clipbox streaming system with threaded update (thanks to lenesxy, issue #692) - Fixed blocks were saved with incorrect LOD index when they get unloaded using Clipbox, leading to holes and mismatched terrain (#691) - Fixed incorrect loading of chunks near terrain borders when viewers are far away from bounds, when using the Clipbox streaming system + - `VoxelStreamSQLite`: fixed connection leaks (thanks to lenesxy, issue #713) - `VoxelTerrain`: edits and copies across fixed bounds no longer behave as if terrain generates beyond (was causing "walls" to appear). - `VoxelGeneratorGraph`: - Fixed wrong values when using `OutputWeight` with optimized execution map enabled, when weights are determined to be locally constant diff --git a/streams/sqlite/voxel_stream_sqlite.cpp b/streams/sqlite/voxel_stream_sqlite.cpp index cbf1e1d5d..3bd6000f9 100644 --- a/streams/sqlite/voxel_stream_sqlite.cpp +++ b/streams/sqlite/voxel_stream_sqlite.cpp @@ -111,6 +111,7 @@ void VoxelStreamSQLite::load_voxel_blocks(Span p_bl // This should be quick after the first call because the connection is cached. sqlite::Connection *con = get_connection(); ERR_FAIL_COND(con == nullptr); + const ScopeRecycle con_scope(this, con); // Check the cache first StdVector blocks_to_load; @@ -133,7 +134,6 @@ void VoxelStreamSQLite::load_voxel_blocks(Span p_bl if (blocks_to_load.size() == 0) { // Everything was cached, no need to query the database - recycle_connection(con); return; } @@ -161,16 +161,16 @@ void VoxelStreamSQLite::load_voxel_blocks(Span p_bl } ERR_FAIL_COND(con->end_transaction() == false); - - recycle_connection(con); } void VoxelStreamSQLite::save_voxel_blocks(Span p_blocks) { sqlite::Connection *con = get_connection(); + ZN_ASSERT_RETURN(con != nullptr); const BlockLocation::CoordinateFormat coordinate_format = con->get_meta().coordinate_format; + recycle_connection(con); + const Box3i coordinate_range = BlockLocation::get_coordinate_range(coordinate_format); const unsigned int lod_count = BlockLocation::get_lod_count(coordinate_format); - recycle_connection(con); // First put in cache for (unsigned int i = 0; i < p_blocks.size(); ++i) { @@ -223,9 +223,9 @@ void VoxelStreamSQLite::load_instance_blocks(Spanbegin_transaction() == false); for (unsigned int i = 0; i < blocks_to_load.size(); ++i) { @@ -260,16 +260,16 @@ void VoxelStreamSQLite::load_instance_blocks(Spanend_transaction() == false); - - recycle_connection(con); } void VoxelStreamSQLite::save_instance_blocks(Span p_blocks) { sqlite::Connection *con = get_connection(); + ZN_ASSERT_RETURN(con != nullptr); const BlockLocation::CoordinateFormat coordinate_format = con->get_meta().coordinate_format; + recycle_connection(con); + const Box3i coordinate_range = BlockLocation::get_coordinate_range(coordinate_format); const unsigned int lod_count = BlockLocation::get_lod_count(coordinate_format); - recycle_connection(con); // First put in cache for (size_t i = 0; i < p_blocks.size(); ++i) { @@ -296,6 +296,7 @@ void VoxelStreamSQLite::load_all_blocks(FullLoadingResult &result) { sqlite::Connection *con = get_connection(); ERR_FAIL_COND(con == nullptr); + const ScopeRecycle con_scope(this, con); struct Context { FullLoadingResult &result; @@ -362,8 +363,8 @@ int VoxelStreamSQLite::get_used_channels_mask() const { void VoxelStreamSQLite::flush_cache() { sqlite::Connection *con = get_connection(); ERR_FAIL_COND(con == nullptr); + const ScopeRecycle con_scope(this, con); flush_cache_to_connection(con); - recycle_connection(con); } void VoxelStreamSQLite::flush() { @@ -509,10 +510,11 @@ VoxelStreamSQLite::CoordinateFormat VoxelStreamSQLite::get_preferred_coordinate_ } VoxelStreamSQLite::CoordinateFormat VoxelStreamSQLite::get_current_coordinate_format() { - const sqlite::Connection *con = get_connection(); + sqlite::Connection *con = get_connection(); if (con == nullptr) { return get_preferred_coordinate_format(); } + const ScopeRecycle con_scope(this, con); return to_exposed_coordinate_format(con->get_meta().coordinate_format); } @@ -524,9 +526,6 @@ bool VoxelStreamSQLite::copy_blocks_to_other_sqlite_stream(Refget_database_path() != get_database_path(), false); ZN_ASSERT_RETURN_V_MSG( @@ -535,6 +534,10 @@ bool VoxelStreamSQLite::copy_blocks_to_other_sqlite_stream(Refget_connection(); + ZN_ASSERT_RETURN_V(context.dst_con != nullptr, false); + const ScopeRecycle dst_con_scope(dst_stream.ptr(), context.dst_con); + + const bool success = src_con->load_all_blocks(&context, Context::save); - return src_con->load_all_blocks(&context, Context::save); + return success; } void VoxelStreamSQLite::_bind_methods() { diff --git a/streams/sqlite/voxel_stream_sqlite.h b/streams/sqlite/voxel_stream_sqlite.h index 3b56a7aa7..d67810bc1 100644 --- a/streams/sqlite/voxel_stream_sqlite.h +++ b/streams/sqlite/voxel_stream_sqlite.h @@ -128,6 +128,24 @@ class VoxelStreamSQLite : public VoxelStream { sqlite::Connection *get_connection(); void recycle_connection(sqlite::Connection *con); + + struct ScopeRecycle { + VoxelStreamSQLite *stream; + sqlite::Connection *connection; + + ScopeRecycle(VoxelStreamSQLite *p_stream, sqlite::Connection *p_connection) : + stream(p_stream), connection(p_connection) { +#ifdef DEV_ENABLED + ZN_ASSERT(stream != nullptr); + ZN_ASSERT(connection != nullptr); +#endif + } + + ~ScopeRecycle() { + stream->recycle_connection(connection); + } + }; + void flush_cache_to_connection(sqlite::Connection *p_connection); static void _bind_methods();