JDK-8054039 Deadlock during interrupting FileChannel

[Frank Yuan]

Hi Brian and Daniel

 

Any comment or any concern for this patch?

 

Thanks

Frank

 

From: Frank Yuan [mailto:frank.yuan at oracle.com] 
Sent: Tuesday, August 23, 2016 11:26 AM
To: 'Brian Burkhalter'; 'Daniel Fuchs'; 'nio-dev'
Subject: RE: JDK-8054039 Deadlock during interrupting FileChannel

 

Does anybody have any comments?

 

Frank

 

From: Frank Yuan [mailto:frank.yuan at oracle.com] 
Sent: Wednesday, August 17, 2016 6:21 PM
To: 'Brian Burkhalter'; 'Daniel Fuchs'; 'nio-dev'
Subject: RE: JDK-8054039 Deadlock during interrupting FileChannel

 

Hi 

 

I reclaimed this task after a long time of suspending(really sorry for that.)

 

However, I am sure for the fix now, that is the solution 1 which we talked about. I made an update for the latest code, would you
like to check again: http://cr.openjdk.java.net/~fyuan/8054039/webrev.00/ ?

 

As you known, this failure scenario is like:

Thread 1 hold blockerLock of thread 2 and try to hold closeLock of FileChannel 1

Thread 2 hold closeLock of FileChannel 1 and try to hold blockerLock of thread 2, i.e. its own thread

 

To solve the deadlock, this patch won't try to hold blockerLock of current thread in FileChannel, look at the change in
NativeThreadSet.java

         if (interrupted)

-            Thread.currentThread().interrupt();

+            SharedSecrets.getJavaLangAccess().doInterrupt(Thread.currentThread());

     }

Here, it only need to set the interrupted flag, doesn't need to call Interruptible.interrupt(<current thread>)  extra, because
either <current thread>.interrupt() will call it(since current thread is already interrupted its interrupt method must be already
invoked) or the AbstractInterruptibleChannel object will call it in the begin method(because we have set the interrupted flag again)
if current thread has an Interruptible object.

 

 

Btw, I am not sure if we need to add any note in the comment of AbstractInterruptibleChannel.implCloseChannel() to state the
implementation must avoid to call interrupt method of current thread.

 

Thanks

Frank

 

 

 

From: Frank Yuan [mailto:frank.yuan at oracle.com] 
Sent: Thursday, September 10, 2015 5:04 PM
To: 'Brian Burkhalter'; 'nio-dev'
Cc: 'FELIX YANG'
Subject: RE: JDK-8054039 Deadlock during interrupting FileChannel

 

 

 

Hi Frank,

 

On Sep 9, 2015, at 12:59 AM, Frank Yuan <frank.yuan at oracle.com <mailto:frank.yuan at oracle.com> > wrote:

 

Thank you for review and your comments:)

 

You are welcome.

 

The size of my last mail is too large, is still held by maillist moderator, I have to send some content again(the diagram still
can't be attached.).

 

You could upload the diagram and a webrev to cr.openjdk.java.net <http://cr.openjdk.java.net>  assuming you have an ID. If not, I
could do it for you.

 

I have uploaded the diagram to the bug https://bugs.openjdk.java.net/browse/JDK-8054039, hope more people can review it.

 

 

I am suspicious of the use of reflection here.

I also have some concern for this solution, because in this fix nio will depend on the private method of Thread -- interrupt0, it
will break the encapsulation. However it's an available solution in theory.

 

True but I think the second one is better.

 

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 my understood, this solution won't spend more time on begin() than the old code, compareAndSet is atomic and there is no loop or
wait. Anyway I am not sure your concern, will do more tests, hope more people can review it.

 

Indeed I don't know whether there would be any effect: it was simply an observation. I expect others will comment when they have
time.

 

Brian

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160831/b8992ad6/attachment-0001.html>