You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "rishabhdaim (via GitHub)" <gi...@apache.org> on 2026/01/29 05:56:45 UTC
[PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
rishabhdaim opened a new pull request, #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711
(no comment)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#issuecomment-3818535755
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741622300
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
Review Comment:
With this timing, joiningThread will have terminated already. The assertion in joiningThread will have failed. The test passes because that AssertionError doesn't happen in the main thread.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741616468
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtils.java:
##########
@@ -110,4 +110,39 @@ public static void sleepUninterruptibly(final long sleep, final TimeUnit unit) {
}
}
}
+
+ /**
+ * Invokes {@link TimeUnit#timedJoin(Thread, long)} uninterruptibly.
+ * <p>
+ * This method repeatedly calls {@link TimeUnit#timedJoin(Thread, long)} until the
+ * specified timeout has elapsed or the target thread terminates, ignoring
+ * {@link InterruptedException} but remembering that an interruption
+ * occurred. When the method finally returns, it restores the current
+ * thread's interrupted status if any interruptions were detected.
+ *
+ * @param toJoin the thread to wait for; must not be {@code null}
+ * @throws NullPointerException if {@code toJoin} or {@code unit} is {@code null}
+ * @throws IllegalArgumentException if {@code timeout} is negative
+ */
+ public static void joinUninterruptibly(final Thread toJoin) {
+
+ Objects.requireNonNull(toJoin, "thread to join is null");
+
+ boolean interrupted = false;
+
+ try {
+ for(;;) {
Review Comment:
maybe `while(true)`?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#issuecomment-3819782027
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711) **Quality Gate passed**
Issues
 [15 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741447027
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,112 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ // Run longer than the join timeout
Review Comment:
addressed in https://github.com/apache/jackrabbit-oak/pull/2711/commits/ebcc1b045e856e517ea50b12681324e2419f00dd
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,112 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ // Run longer than the join timeout
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ // Worker may still be alive (likely), but we at least checked the timeout behavior
Review Comment:
addressed in https://github.com/apache/jackrabbit-oak/pull/2711/commits/ebcc1b045e856e517ea50b12681324e2419f00dd
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741632996
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
Review Comment:
any suggestion to fix this ?
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
Review Comment:
any suggestion to fix this ?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim merged PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741300878
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,112 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ // Run longer than the join timeout
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ // Worker may still be alive (likely), but we at least checked the timeout behavior
Review Comment:
I don't understand. The whole point of join() is to wait until the thread terminates. How would it still be alive after join() returns?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#issuecomment-3816027290
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711) **Quality Gate failed**
Failed conditions
 [54.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711&metric=new_coverage&view=list) (required ≥ 80%)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2711)
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741636323
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
Review Comment:
would removing the `sleep` fix this issue ?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741622082
##########
oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtils.java:
##########
@@ -110,4 +110,39 @@ public static void sleepUninterruptibly(final long sleep, final TimeUnit unit) {
}
}
}
+
+ /**
+ * Invokes {@link TimeUnit#timedJoin(Thread, long)} uninterruptibly.
+ * <p>
+ * This method repeatedly calls {@link TimeUnit#timedJoin(Thread, long)} until the
+ * specified timeout has elapsed or the target thread terminates, ignoring
+ * {@link InterruptedException} but remembering that an interruption
+ * occurred. When the method finally returns, it restores the current
+ * thread's interrupted status if any interruptions were detected.
+ *
+ * @param toJoin the thread to wait for; must not be {@code null}
+ * @throws NullPointerException if {@code toJoin} or {@code unit} is {@code null}
+ * @throws IllegalArgumentException if {@code timeout} is negative
+ */
+ public static void joinUninterruptibly(final Thread toJoin) {
+
+ Objects.requireNonNull(toJoin, "thread to join is null");
+
+ boolean interrupted = false;
+
+ try {
+ for(;;) {
Review Comment:
They are same, even at the machine code level.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741302707
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,112 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ // Run longer than the join timeout
Review Comment:
which timeout?
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2741631591
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
+ // Interrupt while it is joining
+ joiningThread.interrupt();
+
+ joiningThread.join(200L);
+ }
+
+ @Test
+ public void testJoinUninterruptiblyMultipleInterruptsStillCompleteAndRestoreFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(30L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ Assert.assertTrue("Interrupt flag should be restored after multiple interrupts",
+ Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Interrupt the joining thread multiple times while it is waiting
+ for (int i = 0; i < 3; i++) {
+ Thread.sleep(50L);
Review Comment:
Same as above, too late to interrupt.
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-12077 : added joinUninterruptibly() in oak-commons [jackrabbit-oak]
Posted by "rishabhdaim (via GitHub)" <gi...@apache.org>.
rishabhdaim commented on code in PR #2711:
URL: https://github.com/apache/jackrabbit-oak/pull/2711#discussion_r2742730954
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
+ // Interrupt while it is joining
+ joiningThread.interrupt();
+
+ joiningThread.join(200L);
+ }
+
+ @Test
+ public void testJoinUninterruptiblyMultipleInterruptsStillCompleteAndRestoreFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(30L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ Assert.assertTrue("Interrupt flag should be restored after multiple interrupts",
+ Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Interrupt the joining thread multiple times while it is waiting
+ for (int i = 0; i < 3; i++) {
+ Thread.sleep(50L);
Review Comment:
fixed here : https://github.com/apache/jackrabbit-oak/pull/2711/commits/31eec993e2327c0400f5d4449d3f1fb3c7f0add0
##########
oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/internal/concurrent/UninterruptibleUtilsTest.java:
##########
@@ -162,4 +162,104 @@ public void testZeroSleepReturnsQuickly() {
Assert.assertTrue("Zero sleep should return quickly", elapsedMillis < 50L);
}
+ @Test
+ public void testNullThread() {
+ Assert.assertThrows(NullPointerException.class,
+ () -> UninterruptibleUtils.joinUninterruptibly(null));
+ }
+
+ @Test
+ public void testReturnsWhenThreadFinishesBeforeTimeout() throws Exception {
+ final long workMillis = 10L;
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(workMillis);
+ } catch (InterruptedException ignored) {}
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertFalse("Worker should be finished", worker.isAlive());
+ Assert.assertTrue("Join should not take excessively long",
+ elapsedMillis >= workMillis && elapsedMillis < 100L);
+ }
+
+ @Test
+ public void testJoinShouldWaitUntilThreadFinishes() {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ long start = System.nanoTime();
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
+
+ Assert.assertTrue("Join should respect timeout", elapsedMillis >= 20L );
+ }
+
+ @Test
+ public void testJoinUninterruptiblyIgnoresInterruptsButRestoresFlag() throws Exception {
+ final Thread worker = new Thread(() -> {
+ try {
+ Thread.sleep(20L);
+ } catch (InterruptedException ignored) {
+ }
+ });
+
+ worker.start();
+
+ Thread joiningThread = new Thread(() -> {
+ UninterruptibleUtils.joinUninterruptibly(worker);
+ // After returning, interrupted flag should be set if we interrupted during join
+ Assert.assertTrue("Interrupt flag should be restored", Thread.currentThread().isInterrupted());
+ });
+
+ joiningThread.start();
+
+ // Let the joining thread enter join
+ Thread.sleep(50L);
+
Review Comment:
fixed here : https://github.com/apache/jackrabbit-oak/pull/2711/commits/31eec993e2327c0400f5d4449d3f1fb3c7f0add0
--
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: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org