Skip to content

Commit

Permalink
More careful result destructor
Browse files Browse the repository at this point in the history
Only set the current result to null if we are cleaning
up the current result.

What happens is that when a result is not cleaned up and then you make
another query, then the first result set is not freed, but it is left
dangling, with a reference to the connection:
https://github.com/r-dbi/odbc/blob/a0ba73c72ca8f4cf33181fead7644b9b90aa2d4c/src/odbc_connection.cpp#L19

We test that the first result is invalidated in DBItest, here:
https://github.com/r-dbi/DBItest/blob/2c29217bdd0b3314e6ec65216d99a3bf83a63464/R/spec-result-send-query.R#L92
So after that test there is still a result object, until the garbage
collector runs next.

If the garbage collector happens to run when there is another
active result for the same connection, we remove that result from
the connection, even though it is not the result we are finalizing.

The workaround in this PR check that we are finalizing the active
result in the destructor.

A better solution would be to finalize the old result when a new one
is set, but that seemed more difficult with the current code base.
  • Loading branch information
gaborcsardi committed Oct 9, 2023
1 parent a0ba73c commit 2eba123
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/odbc_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ bool odbc_result::complete() {
bool odbc_result::active() { return c_->is_current_result(this); }

odbc_result::~odbc_result() {
if (c_ != nullptr) {
if (c_ != nullptr && active()) {
try {
c_->set_current_result(nullptr);
} catch (...) {
Expand Down

0 comments on commit 2eba123

Please sign in to comment.