mridulm commented on code in PR #50594:
URL: https://github.com/apache/spark/pull/50594#discussion_r2060457816
##########
core/src/main/scala/org/apache/spark/util/UninterruptibleThread.scala:
##########
@@ -69,10 +75,22 @@ private[spark] class UninterruptibleThread(
}
uninterruptibleLock.synchronized {
+ uninterruptible = true
+ }
+
+ while (uninterruptibleLock.synchronized {
// Clear the interrupted status if it's set.
shouldInterruptThread = Thread.interrupted() || shouldInterruptThread
- uninterruptible = true
+ // wait for super.interrupt() to be called
+ !shouldInterruptThread && awaitInterruptThread }) {
Review Comment:
`shouldInterruptThread` is updated here in this method, and in `interrupt`.
The update within this method is single thread logic, so looking at
interactions with interrupt method overlap:
RU for thread in `runUninterruptibly`
IN for thread in `interrupt`.
We have following cases:
a) RU enters `while` loop before IN enters interrupt.
interrupt thread will return immediately after setting shouldInterruptThread
= true
RU will throw interrupt when method is done (in finally).
b) RU updates `uninterruptible = true`, IN enters `uninterruptibleLock`
before RU enters `while` loop.
We will have:
`shouldInterruptThread = true` (as `uninterruptible = true`)
`awaitInterruptThread = true`
So RU will wait until interrupt is done (when `awaitInterruptThread` becomes
`false`), then proceed - thread has already been interrupted.
Note: any reenterant call in RU will get short circuited at beginning itself
(second `if` condition), and any interrupt call (from the same or different
thread as IN) will be essentially exit immediately in `interrupt` (since
`uninterruptible = true`).
c) IN enters `uninterruptibleLock` before RU updates `uninterruptible = true`
shouldInterruptThread will be set to existing value of `uninterruptible` (so
no change).
`awaitInterruptThread` will be set to `true`
IN will do the interrupt, and then set `awaitInterruptThread = false` in
finally
After IN leaves the synchronized block in `if`, RN will block until
`awaitInterruptThread = false`
If there is a concurrent IN thread - given `awaitInterruptThread = true`, it
will never enter the if block.
+CC @vrozov as well.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
