[
https://issues.apache.org/jira/browse/GEODE-7573?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17000991#comment-17000991
]
Jacob Barrett commented on GEODE-7573:
--------------------------------------
The transactions support in the C++ client is in a very bad place. Any changes
you want to me there are probably fair game. Clearly the documentation is wrong
since TransactionId could never be {{nullptr}}. I would suggest you put
together an RFC outlining what changes you think should be made. Anything short
of changing exported method signatures should be fair game. If changes to the
API are required to make it function I wouldn't oppose those changes either,
they would just need to be introduced carefully.
> 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
> Assignee: Alberto Gomez
> Priority: Major
>
> 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 null. 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)
