SOCI/soci

Using `row::move_as` while iterating `rowset` yields segmentation fault

Closed this issue · 3 comments

Originally reported by @avpalienko in #1141 (comment)

When applying the following patch to the common-tests.h file, the move_as test case will crash due to a segmentation fault:

diff --git a/tests/common-tests.h b/tests/common-tests.h
index d2b8fb42..ffc80fc6 100644
--- a/tests/common-tests.h
+++ b/tests/common-tests.h
@@ -6779,7 +6779,7 @@ TEST_CASE_METHOD(common_tests, "BLOB", "[core][blob]")
         }
         SECTION("move_as")
         {
-            soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id", soci::use(id));
+            soci::rowset< soci::row > rowSet = (sql.prepare << "select b from soci_test where id=:id union all select b from soci_test where id=:id", soci::use(id, "id"));
             bool containedData = false;
             for (auto it = rowSet.begin(); it != rowSet.end(); ++it)
             {

The issue was originally reported to happen with Oracle, but I was also able to reproduce it with SQLite3, indicating that the issue is not backend-specific.

The problem seems to boil down to this:

  • the row object inside the rowset seems to be reused across different data fetches (loop iterations)
  • the row object is (seemingly) only initialized when first creating the rowset object (by binding the contained statement to the row in rowset's constructor)
  • when using move_as the respective entry inside row's holders gets into an invalid state. In case of a blob that corresponds to an uninitialized blob object (one where the backend pointer is nullptr)
  • In the next iteration, the statement wants to write data into the previously bound objects, including those that have been moved from and are thus in an invalid state
  • Trying to write data into an uninitialized blob dereferences the nullptr backend resulting in the segmentation fault

Therefore, the fix for this issue would be to find a way to let the row object know about invalidated holders, which should cause it to reinitialize them before trying to use them for data binding.

May be we could use shared_ptr inside the "row" class and eliminate move_as altogether ?

Nah, the get function cant return a pointer. That'd be inconsistent.
Besides, this wouldn't really solve the underlying problem. If the user moves from the dereferenced pointer, we're back at square one with the row object being broken.