HyukjinKwon commented on code in PR #59061:
URL: https://github.com/apache/spark/pull/59061#discussion_r4172216633
##########
core/src/test/scala/org/apache/spark/util/UtilsSuite.scala:
##########
@@ -572,6 +573,37 @@ class UtilsSuite extends SparkFunSuite with
ResetSystemProperties {
assert(!Utils.createDirectory(scenario8))
}
+ test("chmod700 leaves rwx------ under concurrent calls on the same path") {
+ // Workers in local-cluster mode share local root dirs and can chmod the
same
+ // per-application directory concurrently; the stat-then-chmod File.set*
sequence
+ // interleaves into broken permission sets, so chmod700 must be atomic.
+
assume(FileSystems.getDefault.supportedFileAttributeViews().contains("posix"))
+ withTempDir { dir =>
+ // Start from a permissive mode so the assertion cannot pass vacuously
under umask 077.
+ Files.setPosixFilePermissions(dir.toPath,
PosixFilePermissions.fromString("rwxr-xr-x"))
+ (1 to 10).foreach { _ =>
+ val threads = (1 to 4).map { _ =>
+ new Thread(() => (1 to 20).foreach(_ => Utils.chmod700(dir)))
+ }
+ threads.foreach(_.start())
+ threads.foreach(_.join())
+ assert(
Review Comment:
**Nit (P3):** This assertion runs only after every thread has joined, and
the post-join mode is deterministically `rwx------` whether or not chmod700 is
atomic: each thread completes all its calls, and each call ends on owner
grants, so for every permission bit the globally last operation is a grant. A
revert to the old non-atomic `File.set*` sequence would still pass this test,
so it does not actually guard the atomic-vs-non-atomic property it is named for
(it does still guard that chmod700 lands at `rwx------`). The `CyclicBarrier`
suggested in the other thread would not change this either, since it only
aligns thread starts while the assertion still samples the quiescent post-join
state. To catch the transient broken state you would need to sample permissions
from an observer thread during the race; otherwise consider reframing this as
an end-state check and renaming it so the intent matches what it verifies.
**Verification:**
- **Regression:** A concurrent chmod700 scenario whose assertion fails
whenever the final directory mode is anything other than rwx------.
- **Inspection:** The test name and comment no longer claim to detect the
non-atomic mid-race interleaving that a post-join assertion cannot observe.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
