You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "YanYunyang (via GitHub)" <gi...@apache.org> on 2024/07/05 07:49:47 UTC
[PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
YanYunyang opened a new pull request, #8367:
URL: https://github.com/apache/rocketmq/pull/8367
… a channel, no longer acquire a read lock.
<!-- Please make sure the target branch is right. In most case, the target branch should be `develop`. -->
### Which Issue(s) This PR Fixes
<!-- Please ensure that the related issue has already been created, and [link this pull request to that issue using keywords](<https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword>) to ensure automatic closure. -->
Fixes #8366
### Brief Description
<img width="858" alt="截屏2024-07-05 15 38 50" src="https://github.com/apache/rocketmq/assets/11780013/9414ae94-a62c-43fd-9f43-d4da1d25483a">
`ChannelWrapper.getChannel()` needs to hold the read lock. When determining if `ChannelWrapper` is the wrapper for a channel, no longer acquire a read lock.
<!-- Write a brief description for your pull request to help the maintainer understand the reasons behind your changes. -->
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "drpmma (via GitHub)" <gi...@apache.org>.
drpmma commented on PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367#issuecomment-2222100844
> https://github.com/apache/rocketmq/blob/32f0e8481d1f5672b8d494de7607fe5b846ffa51/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java#L479-L484
>
> It seems that this piece of code also may lead to deadlock.
how about tryClose method which contains lock.readLock().lock(), it also may lead to deadlock.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "RongtongJin (via GitHub)" <gi...@apache.org>.
RongtongJin merged PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "YanYunyang (via GitHub)" <gi...@apache.org>.
YanYunyang commented on PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367#issuecomment-2221957851
> There are two closeChannel method.
>
> This piece of code also needs modification.
>
> https://github.com/apache/rocketmq/blob/32f0e8481d1f5672b8d494de7607fe5b846ffa51/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java#L424-L428
Here it needs to be changed to `isWrapperOf` to match the meaning. Thank you.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "drpmma (via GitHub)" <gi...@apache.org>.
drpmma commented on PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367#issuecomment-2219815886
https://github.com/apache/rocketmq/blob/32f0e8481d1f5672b8d494de7607fe5b846ffa51/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java#L479-L484
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367#issuecomment-2222144132
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/8367?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 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`)](https://app.codecov.io/gh/apache/rocketmq/commit/77d6633e622200ed52feef6fb2adeb12fba8e2c8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`36d561f`)](https://app.codecov.io/gh/apache/rocketmq/commit/36d561f118de4eee06a9bea720dc1fae44d42e72?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 5 commits behind head on develop.
| [Files](https://app.codecov.io/gh/apache/rocketmq/pull/8367?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/rocketmq/remoting/netty/NettyRemotingClient.java](https://app.codecov.io/gh/apache/rocketmq/pull/8367?src=pr&el=tree&filepath=remoting%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fremoting%2Fnetty%2FNettyRemotingClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdDbGllbnQuamF2YQ==) | 75.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/8367?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/rocketmq/pull/8367?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #8366] Eliminate deadlocks during the client shutdown process. [rocketmq]
Posted by "YanYunyang (via GitHub)" <gi...@apache.org>.
YanYunyang commented on PR #8367:
URL: https://github.com/apache/rocketmq/pull/8367#issuecomment-2221953973
> https://github.com/apache/rocketmq/blob/32f0e8481d1f5672b8d494de7607fe5b846ffa51/remoting/src/main/java/org/apache/rocketmq/remoting/netty/NettyRemotingClient.java#L479-L484
>
> It seems that this piece of code also may lead to deadlock.
Here is the same channelWrapper. The lock in channelWrapper is reentrant.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org