Skip to content

Commit 7663fdf

Browse files
committed
[SPARK-45499][CORE][TESTS][FOLLOWUP] Use ReferenceQueue#remove instead of ReferenceQueue#poll
### What changes were proposed in this pull request? This pr replaces `refQueue.poll()` with `refQueue.remove()` in the test case `reference to sub iterator should not be available after completion` to ensure that a `PhantomReference` object can be retrieved from `refQueue`. ### Why are the changes needed? #43325 replaces `Reference#isEnqueued` with `Reference#refersTo(null)` to eliminate the use of deprecated APIs. However, there are some differences between `ref.isEnqueued` and `ref.refersTo(null)`. - The `ref.isEnqueued` method is used to check whether this `PhantomReference` object has been added to its reference queue by the garbage collector. When the garbage collector decides to recycle an object, if this object has one or more `PhantomReference`, then these `PhantomReference` will be added to their reference queues. So, if `ref.isEnqueued` returns `true`, it means that this `PhantomReference` has been added to the reference queue, which means that the object it references has been recycled by the garbage collector. - The `ref.refersTo(null)` method is used to check whether this `PhantomReference` object refers to the specified object. In the current code, `ref.refersTo(null)` is used to check whether `ref` still refers to `sub`. If `ref.refersTo(null)` returns `true`, it means that `ref` no longer refers to `sub`, which means that `sub` might have been recycled by the garbage collector. But this does not mean that this `ref` has been added to the reference queue. So we can see the following test failure in GA: https://github.com/apache/spark/actions/runs/6484510414/job/17608536854 ``` [info] - reference to sub iterator should not be available after completion *** FAILED *** (287 milliseconds) [info] null did not equal java.lang.ref.PhantomReference11e8f090 (CompletionIteratorSuite.scala:67) [info] org.scalatest.exceptions.TestFailedException: [info] at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472) [info] at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471) [info] at org.scalatest.Assertions$.newAssertionFailedException(Assertions.scala:1231) [info] at org.scalatest.Assertions$AssertionsHelper.macroAssert(Assertions.scala:1295) [info] at org.apache.spark.util.CompletionIteratorSuite.$anonfun$new$3(CompletionIteratorSuite.scala:67) ``` To solve this issue, this PR replaces `refQueue.poll()` with `refQueue.remove()` to allow for waiting until `ref` is put into `refQueue` and can be retrieved from `refQueue`. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Pass GitHub Actions ### Was this patch authored or co-authored using generative AI tooling? No Closes #43345 from LuciferYang/ref-remove. Authored-by: yangjie01 <yangjie01@baidu.com> Signed-off-by: yangjie01 <yangjie01@baidu.com>
1 parent b0576ff commit 7663fdf

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

core/src/test/scala/org/apache/spark/util/CompletionIteratorSuite.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,6 @@ class CompletionIteratorSuite extends SparkFunSuite {
6464
}
6565
}
6666
assert(ref.refersTo(null))
67-
assert(refQueue.poll() === ref)
67+
assert(refQueue.remove(1000) === ref)
6868
}
6969
}

0 commit comments

Comments
 (0)