[GitHub] [flink] zhijiangW commented on a change in pull request #12912: [FLINK-18595][network] Fix the deadlock issue by task thread and canceler thread in RemoteInputChannel

[GitBox <gi...@apache.org>]

zhijiangW commented on a change in pull request #12912:
URL: https://github.com/apache/flink/pull/12912#discussion_r457013772



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/RemoteInputChannel.java
##########
@@ -356,6 +356,13 @@ public Buffer getNextReceivedBuffer() {
 	@Override
 	public NotificationResult notifyBufferAvailable(Buffer buffer) {
 		NotificationResult notificationResult = NotificationResult.BUFFER_NOT_USED;
+		// Two remote channels might call this method mutually by task thread and canceller thread concurrently.
+		// To avoid deadlock issue we can check the released state to return immediately before synchronizing.
+		// See FLINK-18595 for details.
+		if (isReleased.get()) {
+			return notificationResult;

Review comment:
       Before the change, if the channel was already released while calling `notifyBufferAvailable`, it will exit directly by the following: 
   
   ```
   if (isReleased.get() || bufferQueue.getAvailableBufferSize() >= numRequiredBuffers) {
   	isWaitingForFloatingBuffers = false;
   	return notificationResult;
   }
   ```
   
   So we will not leave it as true finally. 
   
   Another option is to explicitly reset this state as false in `#releaseAllResources`, but it would also bring some additional check while checking the `isWaitingForFloatingBuffers` state to avoid misleading exception.  Even we also need to remove listeners from `LocalBufferPool` to keep consistent state. Otherwise the channel is not waiting for floating buffers, but it is still keeping as listener inside `LocalBufferPool` to bring conflict. So in the past we took the way of delay handling this state together while `notifyBufferAvailable`.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org