-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48187/#review136004
-----------------------------------------------------------




geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXManagerImpl.java 
(line 1024)
<https://reviews.apache.org/r/48187/#comment201009>

    failoverMap is synchronized but this code does not look threadsafe. Between 
the call of "containsKey" and "remove" another thread might do a remove of a 
non-null value but has not yet put it back in. Then this thread does its remove 
and sees it return null and does a put of null.
    
    I think you should bracket your containsKey, remove, and put calls with:
    synchronized (failoverMap) {
    }



geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXManagerImpl.java 
(line 1028)
<https://reviews.apache.org/r/48187/#comment201008>

    The comment talks about moving the txid to the front of the queue.
    I would add that is does this by removing and putting the txid back into 
the Linked map.


- Darrel Schneider


On June 2, 2016, 2:33 p.m., Eric Shu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48187/
> -----------------------------------------------------------
> 
> (Updated June 2, 2016, 2:33 p.m.)
> 
> 
> Review request for geode, Darrel Schneider and Swapnil Bawaskar.
> 
> 
> Bugs: GEODE-1491
>     https://issues.apache.org/jira/browse/GEODE-1491
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Make sure when checking if a transaction is completed from 
> isHostedTxRecentlyCompleted() method, the rolled back transaction is 
> considered as well.
> Added a unit test that would fail without the fix and pass with the fix.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXManagerImpl.java
>  df0176d 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/partitioned/PartitionMessage.java
>  9c54587 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/command/RollbackCommand.java
>  ed7c706 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/TXManagerImplTest.java
>  a4b8127 
> 
> Diff: https://reviews.apache.org/r/48187/diff/
> 
> 
> Testing
> -------
> 
> precheckin.
> 
> 
> Thanks,
> 
> Eric Shu
> 
>

Reply via email to