Alberto Gomez created GEODE-7573:
------------------------------------
Summary: Issues with TrasactionIds managed by
CacheTransactionManager in C++ native client
Key: GEODE-7573
URL: https://issues.apache.org/jira/browse/GEODE-7573
Project: Geode
Issue Type: Bug
Components: native client
Reporter: Alberto Gomez
There are several problems related to the TransactionIds managed by the
CacheTransactionManager class:
On the one hand, according to the documentation, the
CacheTransactionManager::getTransactionId() returns null if no transaction is
associated to the thread but according to the signature of the method the
object returned is of type TransactionId&. Therefore, there is no possibility
to return true. The same applies to the
CacheTransactionManager::getTransactionId() method.
If we go to the implementation classes, the following is observed:
If CacheTransactionManagerImpl::suspend() is invoked and there is no
transaction in progress a TransactionException is thrown. This must be
documented instead of the current information that states that a null pointer
is returned.
If CacheTransactionManagerImpl::getTransactionId() is invoked and there is no
transaction in progress what we get is a segmentation fault because the TXState
that should provide the TransactionId object is null. In my opinion the code
should be changed to throw an exception just as it is done when suspend() is
invoked and there is no transaction in progress.
On the other hand, once a transaction has been commited, a valid TransactionId
reference returned previously by either suspend() or getTransactionId() becomes
invalid because the commit of the transaction deletes the object (which is
stored in the TXState that is destroyed at commit).
Subsequent uses of the TransacionId once the transaction is commited like it is
done in the testThinClientTransactionsWithSticky integration test (for example
calling exists() or resume()) would access freed memory.
Unawareness of this may cause unexpected behavior in the client code and should
be avoided. Two alternatives are proposed:
* Document in the C++ API that TransactionId references returned by
CacheTransactionManager should not be used after the transaction is commited.
* Change the type of object returned/managed to a TransactionId shared pointer.
The problem with the second approach is that it involves a change in the API so
the first alternative is the recommended one.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
