You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by "rickyma (via GitHub)" <gi...@apache.org> on 2024/02/22 19:39:59 UTC
[PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
rickyma opened a new pull request, #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536
### What changes were proposed in this pull request?
Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData().
### Why are the changes needed?
A follow-up PR for: https://github.com/apache/incubator-uniffle/pull/1472
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Existing UTs.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#discussion_r1500167665
##########
client/src/main/java/org/apache/uniffle/client/impl/ShuffleReadClientImpl.java:
##########
@@ -269,6 +269,10 @@ private int read() {
// because PlatformDependent.freeDirectBuffer can only release the ByteBuffer with cleaner.
if (sdr != null) {
sdr.release();
+ // We set sdr to null here to prevent IllegalReferenceCountException that could occur
+ // if sdr.release() is called multiple times in the close() method,
+ // when an exception is thrown by clientReadHandler.readShuffleData().
Review Comment:
> when an exception is thrown by clientReadHandler.readShuffleData().
An exception is thrown from line 277. So `sdr` will not be reassigned successfully.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#discussion_r1500157074
##########
client/src/main/java/org/apache/uniffle/client/impl/ShuffleReadClientImpl.java:
##########
@@ -269,6 +269,10 @@ private int read() {
// because PlatformDependent.freeDirectBuffer can only release the ByteBuffer with cleaner.
if (sdr != null) {
sdr.release();
+ // We set sdr to null here to prevent IllegalReferenceCountException that could occur
+ // if sdr.release() is called multiple times in the close() method,
+ // when an exception is thrown by clientReadHandler.readShuffleData().
Review Comment:
I can't get why this will trigger the issue. sdr will be reassign in the Line 277.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston merged PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on code in PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#discussion_r1500167665
##########
client/src/main/java/org/apache/uniffle/client/impl/ShuffleReadClientImpl.java:
##########
@@ -269,6 +269,10 @@ private int read() {
// because PlatformDependent.freeDirectBuffer can only release the ByteBuffer with cleaner.
if (sdr != null) {
sdr.release();
+ // We set sdr to null here to prevent IllegalReferenceCountException that could occur
+ // if sdr.release() is called multiple times in the close() method,
+ // when an exception is thrown by clientReadHandler.readShuffleData().
Review Comment:
> when an exception is thrown by clientReadHandler.readShuffleData().
An exception is thrown from line 277.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "rickyma (via GitHub)" <gi...@apache.org>.
rickyma commented on PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#issuecomment-1960693037
CI is unstable, I don't think the failure is related to this PR.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#issuecomment-1960189546
## Test Results
2 432 files - 5 2 432 suites - 5 4h 39m 36s :stopwatch: - 2m 44s
823 tests ±0 821 :white_check_mark: ±0 1 :zzz: ±0 1 :x: +1
9 737 runs - 5 9 722 :white_check_mark: - 5 14 :zzz: ±0 1 :x: +1
For more details on these failures, see [this check](https://github.com/apache/incubator-uniffle/runs/21880985585).
Results for commit fc848713. ± Comparison against base commit 59aa30d3.
[test-results]:data:application/gzip;base64,H4sIAISo12UC/1XMyw7CIBCF4VdpWLuQmZYBX8Ygl4TYi6GwMr67tCkVl+dj+N/Mh9Gt7NZBj3Dp2JpDarfNUaewzEW4IBKFynvaLiRgXfc1G7MT/9EzvLZfJ3gdxj9wMS6xyLVIzPPWVIR0rNpUBHDS0ewrNM19t0mzTFNIZTBvZC+Jo1YWjXvYQchBkTVegJSOAAfQiiOwzxfkCrZqEAEAAA==
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
Re: [PR] [#1472][FOLLOWUP] fix(client): Fix IllegalReferenceCountException issues when exceptions happened in clientReadHandler.readShuffleData() [incubator-uniffle]
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on code in PR #1536:
URL: https://github.com/apache/incubator-uniffle/pull/1536#discussion_r1500192885
##########
client/src/main/java/org/apache/uniffle/client/impl/ShuffleReadClientImpl.java:
##########
@@ -269,6 +269,10 @@ private int read() {
// because PlatformDependent.freeDirectBuffer can only release the ByteBuffer with cleaner.
if (sdr != null) {
sdr.release();
+ // We set sdr to null here to prevent IllegalReferenceCountException that could occur
+ // if sdr.release() is called multiple times in the close() method,
+ // when an exception is thrown by clientReadHandler.readShuffleData().
Review Comment:
OK. I know.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org