tinkerpop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From greedy <...@git.apache.org>
Subject [GitHub] incubator-tinkerpop pull request: Fix TINKERPOP-1252
Date Thu, 07 Apr 2016 12:33:21 GMT
Github user greedy commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/288#issuecomment-206860135
  
    Yeah, that's what I saw happening. threadLocalTx.remove() was skipped because threadLocalTx.get().close()
failed.
    
    I did think about using two levels of finally blocks to guarantee that close is called.
The Neo4j API documentation doesn't really say much about which of these methods can fail
and in which ways. I figured that if either one of those fail then the transaction should
be considered dead. From looking through the implementations when I was trying to figure out
what was going on, I saw that success() and failure() just set a boolean flag so they shouldn't
fail. I wouldn't have any problem with cascaded finally blocks to make sure everything is
getting called; if that seems better to you, I can update the pull request to do that.
    
    It does indeed impact the 3.1 series as that is where I actually encountered it. It should
be applied to both so I marked both series as affected in the JIRA issue. I wasn't sure if
I should create two pull requests: this one and one for the tp31 branch; or if a committer
would just cherry-pick this fix on to tp31 if/when it lands on master.
    
    I did run the neo4j test suite for the change and they passed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message