[ISSUE #8366] Eliminate deadlocks during the client shutdown process.

[YanYunyang]

… a channel, no longer acquire a read lock.

Which Issue(s) This PR Fixes

Fixes #8366

Brief Description

ChannelWrapper.getChannel() needs to hold the read lock. When determining if ChannelWrapper is the wrapper for a channel, no longer acquire a read lock.

[drpmma]

rocketmq/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java

Lines 479 to 484 in 32f0e84

	ChannelWrapper channelWrapper = this.channelWrapperTables.remove(channel);
	if (channelWrapper != null && channelWrapper.tryClose(channel)) {
	this.channelTables.remove(addrRemote);
	}
	LOGGER.info("closeChannel: the channel[{}] was removed from channel table", addrRemote);
	RemotingHelper.closeChannel(channel);

It seems that this piece of code also may lead to deadlock.

[drpmma]

There are two closeChannel method.

This piece of code also needs modification.

rocketmq/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java

Lines 424 to 428 in 32f0e84

	} else if (prevCW.getChannel() != channel) {
	LOGGER.info("closeChannel: the channel[{}] has been closed before, and has been created again, nothing to do.",
	addrRemote);
	removeItemFromTable = false;
	}

[YanYunyang]

rocketmq/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java

Lines 479 to 484 in 32f0e84

	ChannelWrapper channelWrapper = this.channelWrapperTables.remove(channel);
	if (channelWrapper != null && channelWrapper.tryClose(channel)) {
	this.channelTables.remove(addrRemote);
	}
	LOGGER.info("closeChannel: the channel[{}] was removed from channel table", addrRemote);
	RemotingHelper.closeChannel(channel);

It seems that this piece of code also may lead to deadlock.

Here is the same channelWrapper, and in the same thread. The lock in channelWrapper is reentrant.

[YanYunyang]

There are two closeChannel method.

This piece of code also needs modification.

rocketmq/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java

Lines 424 to 428 in 32f0e84

	} else if (prevCW.getChannel() != channel) {
	LOGGER.info("closeChannel: the channel[{}] has been closed before, and has been created again, nothing to do.",
	addrRemote);
	removeItemFromTable = false;
	}

Here it needs to be changed to isWrapperOf to match the meaning. Thank you.

[drpmma]

LGTM

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 44.25%. Comparing base (77d6633) to head (36d561f).
Report is 5 commits behind head on develop.

Files	Patch %	Lines
...e/rocketmq/remoting/netty/NettyRemotingClient.java	75.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop    #8367      +/-   ##
=============================================
+ Coverage      43.88%   44.25%   +0.37%     
- Complexity     10647    10742      +95     
=============================================
  Files           1274     1274              
  Lines          88939    88941       +2     
  Branches       11432    11432              
=============================================
+ Hits           39033    39365     +332     
+ Misses         44985    44618     -367     
- Partials        4921     4958      +37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[caigy]

LGTM