JDK-8054039 Deadlock during interrupting FileChannel

[Brian Burkhalter]

Hi Frank,

Sorry that this topic has languished for so long with no comment.

On Sep 8, 2015, at 2:47 AM, Frank Yuan <frank.yuan at oracle.com> wrote:

> Thread 1 and thread 2 invoke map() on a same FileChannel, at same time, thread 3 is interrupting thread 2, if it happens to meet the conditions at the 4 time points, thread 2 will hold closeLock of the FileChannel and try to get the blockerLock of itself(i.e. thread 2), thread 3 will hold blockerLock of thread 2 and try to get closeLock of the FileChannel, that will result in the dead lock.

This analysis appears to me to be accurate.

>  With applying attached deadlock.diff, in which I add some delay code at some points, the dead lock happens very easy, refer to the attached <<test InterruptMapDeadlock fail.txt>>, if you use agentvm mode, jtreg will even report the dead lock found, see <<jtreg report dead lock.txt >>
>  
> I have 2 solutions to fix this issue. The first one is to be like the attached fix1.diff, if I understand correctly, in signalAndWait(), Thread.currentThread().interrupt() is just to set interrupted flag again, the whole interrupt() is unnecessary, in my fix1.diff, I invoke Thread. interrupt0() by reflection, this is not the final code, just an example.

I am suspicious of the use of reflection here.

> The second solution is << 8054039.diff >>, which has been attached inhttps://bugs.openjdk.java.net/secure/attachment/28435/8054039.diff. In this fix, I replace closeLock in AbstractInterruptibleChannel.java with AtomicBoolean and compareAndSet(), that eliminates one of the locks, like my solution 1.

Within the scope of AbstractInterruptibleChannel I do not see any problem with the change that you propose. Given that the affected instance variables are private to the class and that AtomicBoolean “generally” follows the rules for volatiles it looks conceptually valid. I do not know however about the implications with respect to interactions with other classes. For example, could this change provoke some unforeseen behavior as a result of subtle changes in timing?

As for picky comments, the source file could use some minor cleanup in the form of removing unused imports and adding @Override annotations where appropriate.

Thanks,

Brian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20150908/1e85a46a/attachment.html>