Skip to content

Commit 18330c6

Browse files
committed
Fix session/result lifetime issues.
We allow session to be closed/deleted while results from that session are still usable. The logic was not fully correct in such a situation. This fix adds more bookkeeping logic to make sure that "orphaned" result does not try to delete or access its session object (which might no longer exist).
1 parent 8093468 commit 18330c6

File tree

3 files changed

+47
-6
lines changed

3 files changed

+47
-6
lines changed

devapi/result.cc

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,20 @@ class internal::BaseResult::Impl
10661066
delete m_reply;
10671067
}
10681068

1069+
/*
1070+
Discard the CDK reply object owned by the implementation. This
1071+
is called when the corresponding session is about to be closed
1072+
and the reply object will be no longer valid.
1073+
*/
1074+
1075+
void discard_reply()
1076+
{
1077+
if (!m_reply)
1078+
return;
1079+
1080+
delete m_reply;
1081+
m_reply = NULL;
1082+
}
10691083

10701084
/*
10711085
Read next row from the cursor. Returns NULL if there are no
@@ -1271,7 +1285,7 @@ internal::BaseResult::BaseResult(XSession_base *sess,
12711285
internal::BaseResult::~BaseResult()
12721286
{
12731287
try {
1274-
if (m_sess)
1288+
if (m_sess && m_sess->m_impl)
12751289
m_sess->deregister_result(this);
12761290
}
12771291
catch (...) {}
@@ -1322,6 +1336,30 @@ internal::BaseResult::get_impl() const
13221336
}
13231337

13241338

1339+
/*
1340+
This method is called when the result object is deregistered from
1341+
the session (so that it is no longer the active result of that
1342+
session).
1343+
1344+
We do cleanups here to make the result object independent from the
1345+
session. Derived classes should cache pending results so that they
1346+
can be accessed without the session.
1347+
*/
1348+
1349+
void internal::BaseResult::deregister_notify()
1350+
{
1351+
assert(m_impl);
1352+
1353+
// Let derived object do its own cleanup
1354+
deregister_cleanup();
1355+
1356+
// Discard CDK reply object which is about to be invalidated.
1357+
m_impl->discard_reply();
1358+
1359+
m_sess = NULL;
1360+
}
1361+
1362+
13251363
unsigned
13261364
internal::BaseResult::getWarningCount() const
13271365
{

devapi/session.cc

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,12 @@ void internal::XSession_base::rollback()
280280
void internal::XSession_base::close()
281281
{
282282
try {
283+
284+
// Results should cache their data before deleting the implementation.
285+
register_result(NULL);
286+
283287
if (m_master_session)
284288
{
285-
// Results should cache their data before deleting the implementation.
286-
register_result(NULL);
287-
288289
//Notify NodeSession nodes that master is being removed.
289290
for (auto node : m_impl->m_nodes)
290291
{

include/devapi/result.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ namespace internal {
313313
}
314314

315315

316-
virtual void deregister_notify(){}
316+
virtual void deregister_cleanup() {}
317+
318+
void deregister_notify();
317319

318320
public:
319321

@@ -792,7 +794,7 @@ class PUBLIC_API RowResult
792794
m_cache = false;
793795
}
794796

795-
void deregister_notify() override
797+
void deregister_cleanup() override
796798
{
797799
//cache elements
798800
count();

0 commit comments

Comments
 (0)