diff --git a/src/fb-cpp/Attachment.cpp b/src/fb-cpp/Attachment.cpp index 9030095..7dadf2d 100644 --- a/src/fb-cpp/Attachment.cpp +++ b/src/fb-cpp/Attachment.cpp @@ -83,7 +83,7 @@ void Attachment::disconnectOrDrop(bool drop) else handle->detach(&statusWrapper); - handle.reset(); + handle.release(); } diff --git a/src/fb-cpp/Batch.cpp b/src/fb-cpp/Batch.cpp index ae0e2bf..87d55e5 100644 --- a/src/fb-cpp/Batch.cpp +++ b/src/fb-cpp/Batch.cpp @@ -227,7 +227,7 @@ void Batch::close() { assert(isValid()); handle->close(&statusWrapper); - handle.reset(); + handle.release(); } FbRef Batch::getInputMetadata() diff --git a/src/fb-cpp/Blob.cpp b/src/fb-cpp/Blob.cpp index 4ef7e5c..0f272f6 100644 --- a/src/fb-cpp/Blob.cpp +++ b/src/fb-cpp/Blob.cpp @@ -238,7 +238,7 @@ void Blob::cancel() assert(isValid()); handle->cancel(&statusWrapper); - handle.reset(); + handle.release(); } void Blob::close() @@ -246,5 +246,5 @@ void Blob::close() assert(isValid()); handle->close(&statusWrapper); - handle.reset(); + handle.release(); } diff --git a/src/fb-cpp/EventListener.cpp b/src/fb-cpp/EventListener.cpp index 13c1cb1..b8abcb9 100644 --- a/src/fb-cpp/EventListener.cpp +++ b/src/fb-cpp/EventListener.cpp @@ -343,7 +343,7 @@ void EventListener::cancelEventsHandle() { // scope std::lock_guard mutexGuard{mutex}; - handle = eventsHandle; + handle = std::move(eventsHandle); } if (!handle) @@ -352,4 +352,5 @@ void EventListener::cancelEventsHandle() StatusWrapper statusWrapper{client}; handle->cancel(&statusWrapper); + handle.release(); } diff --git a/src/fb-cpp/ServiceManager.cpp b/src/fb-cpp/ServiceManager.cpp index 2d41fd0..725a4ee 100644 --- a/src/fb-cpp/ServiceManager.cpp +++ b/src/fb-cpp/ServiceManager.cpp @@ -192,5 +192,5 @@ void ServiceManager::detachHandle() StatusWrapper statusWrapper{*client}; handle->detach(&statusWrapper); - handle.reset(); + handle.release(); } diff --git a/src/fb-cpp/SmartPtrs.h b/src/fb-cpp/SmartPtrs.h index f88de32..0f77501 100644 --- a/src/fb-cpp/SmartPtrs.h +++ b/src/fb-cpp/SmartPtrs.h @@ -61,7 +61,6 @@ namespace fbcpp return FbUniquePtr{obj}; } - // FIXME: Review every usage to see if is not leaking one reference count. /// /// Reference-counted smart pointer for Firebird objects using addRef/release semantics. /// @@ -123,6 +122,18 @@ namespace fbcpp assign(p, false); } + /// + /// Relinquishes ownership of the pointer without releasing it and returns it. + /// Used after Firebird calls that already release the interface on success, like + /// `ITransaction::commit()` or `IAttachment::detach()`. + /// + T* release() noexcept + { + T* tmp = ptr; + ptr = nullptr; + return tmp; + } + FbRef& operator=(FbRef& r) noexcept { assign(r.ptr, true); diff --git a/src/fb-cpp/Statement.cpp b/src/fb-cpp/Statement.cpp index 6db794e..f67445f 100644 --- a/src/fb-cpp/Statement.cpp +++ b/src/fb-cpp/Statement.cpp @@ -238,12 +238,12 @@ void Statement::free() if (resultSetHandle) { resultSetHandle->close(&statusWrapper); - resultSetHandle.reset(); + resultSetHandle.release(); } currentRow = false; statementHandle->free(&statusWrapper); - statementHandle.reset(); + statementHandle.release(); } std::string Statement::getLegacyPlan() @@ -268,7 +268,7 @@ bool Statement::execute(Transaction& transaction) if (resultSetHandle) { resultSetHandle->close(&statusWrapper); - resultSetHandle.reset(); + resultSetHandle.release(); } currentRow = false; diff --git a/src/fb-cpp/Transaction.cpp b/src/fb-cpp/Transaction.cpp index de8e6f4..fb99f3c 100644 --- a/src/fb-cpp/Transaction.cpp +++ b/src/fb-cpp/Transaction.cpp @@ -203,7 +203,7 @@ void Transaction::rollback() StatusWrapper statusWrapper{client}; handle->rollback(&statusWrapper); - handle.reset(); + handle.release(); state = TransactionState::ROLLED_BACK; } @@ -215,7 +215,7 @@ void Transaction::commit() StatusWrapper statusWrapper{client}; handle->commit(&statusWrapper); - handle.reset(); + handle.release(); state = TransactionState::COMMITTED; } diff --git a/src/fb-cpp/request/Request.cpp b/src/fb-cpp/request/Request.cpp index 15ac45b..ab0cf27 100644 --- a/src/fb-cpp/request/Request.cpp +++ b/src/fb-cpp/request/Request.cpp @@ -93,7 +93,7 @@ namespace fbcpp::request assert(isValid()); handle->free(&statusWrapper); - handle.reset(); + handle.release(); } void Request::start(Transaction& transaction, unsigned level) diff --git a/src/test/HandleRelease.cpp b/src/test/HandleRelease.cpp new file mode 100644 index 0000000..99d7b4b --- /dev/null +++ b/src/test/HandleRelease.cpp @@ -0,0 +1,148 @@ +/* + * MIT License + * + * Copyright (c) 2026 Adriano dos Santos Fernandes + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#include "TestUtil.h" +#include "fb-cpp/Blob.h" +#include "fb-cpp/ServiceManager.h" +#include "fb-cpp/Statement.h" +#include "fb-cpp/Transaction.h" + + +BOOST_AUTO_TEST_SUITE(HandleReleaseSuite) + +namespace +{ + // Holds two extra references to a Firebird interface around a call that ends its life. + // released() returns how many references the call gave back and drops the extra ones. + class ReleaseProbe final + { + public: + // Takes the temporary reference returned by getHandle() methods and drops it before counting. + template + explicit ReleaseProbe(FbRef handle) + : ptr{handle.get()} + { + handle.reset(); + + ptr->addRef(); + before = ptr->release(); + ptr->addRef(); + ptr->addRef(); + } + + public: + int released() + { + const int after = ptr->release(); + + if (after > 0) + ptr->release(); + + return before + 1 - after; + } + + private: + fb::IReferenceCounted* ptr; + int before = 0; + }; + + ServiceManagerOptions makeServiceManagerOptions() + { + auto options = ServiceManagerOptions{}; + + if (const auto server = getServer()) + options.setServer(server.value()); + + return options; + } +} // namespace + +BOOST_AUTO_TEST_CASE(handlesReleasedOnce) +{ + const auto database = getTempFile("HandleRelease-handlesReleasedOnce.fdb"); + + Attachment attachment{getClient(), database, AttachmentOptions().setCreateDatabase(true).setForcedWrites(false)}; + + { // scope + Transaction transaction{attachment}; + ReleaseProbe probe{transaction.getHandle()}; + transaction.commit(); + BOOST_CHECK_EQUAL(probe.released(), 1); + } + + { // scope + Transaction transaction{attachment}; + ReleaseProbe probe{transaction.getHandle()}; + transaction.rollback(); + BOOST_CHECK_EQUAL(probe.released(), 1); + } + + { // scope + Transaction transaction{attachment}; + Statement statement{attachment, transaction, "select 1 from rdb$database"}; + + statement.execute(transaction); + ReleaseProbe reexecutedResultSet{statement.getResultSetHandle()}; + statement.execute(transaction); + BOOST_CHECK_EQUAL(reexecutedResultSet.released(), 1); + + ReleaseProbe resultSet{statement.getResultSetHandle()}; + ReleaseProbe stmt{statement.getStatementHandle()}; + statement.free(); + BOOST_CHECK_EQUAL(resultSet.released(), 1); + BOOST_CHECK_EQUAL(stmt.released(), 1); + + Blob closedBlob{attachment, transaction}; + ReleaseProbe closedBlobProbe{closedBlob.getHandle()}; + closedBlob.close(); + BOOST_CHECK_EQUAL(closedBlobProbe.released(), 1); + + Blob cancelledBlob{attachment, transaction}; + ReleaseProbe cancelledBlobProbe{cancelledBlob.getHandle()}; + cancelledBlob.cancel(); + BOOST_CHECK_EQUAL(cancelledBlobProbe.released(), 1); + + transaction.commit(); + } + + { // scope + Attachment second{getClient(), database}; + ReleaseProbe probe{second.getHandle()}; + second.disconnect(); + BOOST_CHECK_EQUAL(probe.released(), 1); + } + + { // scope + ServiceManager manager{getClient(), makeServiceManagerOptions()}; + ReleaseProbe probe{manager.getHandle()}; + manager.disconnect(); + BOOST_CHECK_EQUAL(probe.released(), 1); + } + + ReleaseProbe probe{attachment.getHandle()}; + attachment.dropDatabase(); + BOOST_CHECK_EQUAL(probe.released(), 1); +} + +BOOST_AUTO_TEST_SUITE_END()