> On July 6, 2016, 5:57 p.m., Darrel Schneider wrote:
> > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXLockRequest.java,
> > line 100
> > <https://reviews.apache.org/r/49712/diff/1/?file=1437712#file1437712line100>
> >
> > Why did you remove this try/catch instead of changing it to catch and
> > ignore IllegalArgumentException?
> >
> > Did you figure out how it is possible to have a non-null distLockId but
> > for it to not be in txLockIdList?
> >
> > If we had two threads concurrently call releaseDistributed then that
> > could cause it. Seems like this could happen if one thread is trying to
> > commit or rollback while another thread is trying to close the cache. How
> > about changing releaseDistributed to be a synchronized method? In that case
> > it seems like the IllegalArgumentException would never be expected and you
> > could get rid of all catches of it.
Did you figure out how it is possible to have a non-null distLockId but for it
to not be in txLockIdList?
This is caused by GemfireCacheImpl.close() destroyed the TXLockService --
TXLockService.destroyServices();
The releaseDistributed() call recreated the service without the original
idstLockId.
public void releaseDistributed() {
if (this.distLockId != null) {
TXLockService.createDTLS().release(this.distLockId);
this.distLockId = null;
}
}
One way is to use TXLockService.getDTLS() instead or
TXLockService.createDTLS(), but we have to synchronize on TXLockService static
methods during cache close() to prevend DTLS is nulled out. Do we actually need
to call sychronized metnods just to prevend the non normal cache close case?
The above fix would avoid that.
- Eric
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49712/#review141042
-----------------------------------------------------------
On July 6, 2016, 5:21 p.m., Eric Shu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49712/
> -----------------------------------------------------------
>
> (Updated July 6, 2016, 5:21 p.m.)
>
>
> Review request for geode, Darrel Schneider and Swapnil Bawaskar.
>
>
> Bugs: GEODE-1621
> https://issues.apache.org/jira/browse/GEODE-1621
>
>
> Repository: geode
>
>
> Description
> -------
>
> IllegalArgumentException can be thrown when cache is being closed.
> Rethrow the exception after offheap resources are released.
>
>
> Diffs
> -----
>
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXLockRequest.java
> 06c7572
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/TXState.java
> d64426b
>
> Diff: https://reviews.apache.org/r/49712/diff/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Eric Shu
>
>
