On Thu, Feb 25, 2021 at 4:58 PM <ma...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> markt pushed a commit to branch master
> in repository https://gitbox.apache.org/repos/asf/tomcat.git
>
>
> The following commit(s) were added to refs/heads/master by this push:
> new 35a2e04 Fix SpotBugs warnings
> 35a2e04 is described below
>
> commit 35a2e044d5988466b21b664610ce57b502833cd2
> Author: Mark Thomas <ma...@apache.org>
> AuthorDate: Thu Feb 25 15:57:30 2021 +0000
>
> Fix SpotBugs warnings
> ---
> java/org/apache/catalina/startup/HostConfig.java | 8 +++++---
> res/findbugs/filter-false-positives.xml | 6 ++++++
> .../catalina/nonblocking/TestNonBlockingAPI.java | 18
> +++++++++---------
> 3 files changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/java/org/apache/catalina/startup/HostConfig.java
> b/java/org/apache/catalina/startup/HostConfig.java
> index 6f332b2..b34ea58 100644
> --- a/java/org/apache/catalina/startup/HostConfig.java
> +++ b/java/org/apache/catalina/startup/HostConfig.java
> @@ -1237,6 +1237,7 @@ public class HostConfig implements LifecycleListener
> {
> ExecutorService es = host.getStartStopExecutor();
> List<Future<?>> results = new ArrayList<>();
>
> + // Should not be null as we test above if this is a directory
> String[] migrationCandidates = legacyAppBase.list();
> for (String migrationCandidate : migrationCandidates) {
> File source = new File(legacyAppBase, migrationCandidate);
> @@ -1282,7 +1283,8 @@ public class HostConfig implements LifecycleListener
> {
> tempNew = File.createTempFile("new", null,
> host.getLegacyAppBaseFile());
> tempOld = File.createTempFile("old", null,
> host.getLegacyAppBaseFile());
> // createTempFile is not directly compatible with
> directories, so cleanup
> - tempNew.delete();
> + Files.delete(tempNew.toPath());
> + Files.delete(tempOld.toPath());
>
> // The use of defaults is deliberate here to avoid having to
> // recreate every configuration option on the host. Better to
> change
> @@ -1295,8 +1297,8 @@ public class HostConfig implements LifecycleListener
> {
> migration.execute();
>
> // Use rename
> - destination.renameTo(tempOld);
> - tempNew.renameTo(destination);
> + Files.move(destination.toPath(), tempOld.toPath());
>
Ok, I'm super sorry I didn't retest after this seemingly cosmetic change
until the release vote ... This breaks the migration feature as it now
needs a if (destination.exists()) { } wrapping Files.move, otherwise it
will throw an IOE if the webapp was not previously migrated.
So 10.0.3 is broken. This won't affect 9 and 8.5.
Rémy
> + Files.move(tempNew.toPath(), destination.toPath());
> ExpandWar.delete(tempOld);
>
> } catch (Throwable t) {
> diff --git a/res/findbugs/filter-false-positives.xml
> b/res/findbugs/filter-false-positives.xml
> index e47e128..69cfd78 100644
> --- a/res/findbugs/filter-false-positives.xml
> +++ b/res/findbugs/filter-false-positives.xml
> @@ -532,6 +532,12 @@
> <Bug code="NP" />
> </Match>
> <Match>
> + <!-- legacyAppBase is tested a few lines earlier -->
> + <Class name="org.apache.catalina.startup.HostConfig" />
> + <Method name="migrateLegacyApps" />
> + <Bug pattern="NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE" />
> + </Match>
> + <Match>
> <!-- Domain resolution not an issue here -->
> <Class name="org.apache.catalina.startup.WebappServiceLoader" />
> <Method name="load" />
> diff --git a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
> b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
> index 0ca028b..97c6a22 100644
> --- a/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
> +++ b/test/org/apache/catalina/nonblocking/TestNonBlockingAPI.java
> @@ -1376,9 +1376,9 @@ public class TestNonBlockingAPI extends
> TomcatBaseTest {
>
> private static final long serialVersionUID = 1L;
>
> - private final CountDownLatch responseCommitLatch;
> - private final CountDownLatch clientCloseLatch;
> - private final CountDownLatch asyncCompleteLatch;
> + private final transient CountDownLatch responseCommitLatch;
> + private final transient CountDownLatch clientCloseLatch;
> + private final transient CountDownLatch asyncCompleteLatch;
> private final boolean swallowIoException;
>
> public NBWriteServlet02(CountDownLatch responseCommitLatch,
> CountDownLatch clientCloseLatch,
> @@ -1441,7 +1441,7 @@ public class TestNonBlockingAPI extends
> TomcatBaseTest {
> private final CountDownLatch responseCommitLatch;
> private final CountDownLatch clientCloseLatch;
> private final boolean swallowIoException;
> - private volatile int stage = 0;
> + private volatile AtomicInteger stage = new AtomicInteger(0);
>
> public TestWriteListener02(AsyncContext ac, CountDownLatch
> responseCommitLatch,
> CountDownLatch clientCloseLatch, boolean
> swallowIoException) {
> @@ -1456,12 +1456,12 @@ public class TestNonBlockingAPI extends
> TomcatBaseTest {
> try {
> ServletOutputStream sos =
> ac.getResponse().getOutputStream();
> do {
> - if (stage == 0) {
> + if (stage.get() == 0) {
> // Commit the response
> ac.getResponse().flushBuffer();
> responseCommitLatch.countDown();
> - stage++;
> - } else if (stage == 1) {
> + stage.incrementAndGet();
> + } else if (stage.get() == 1) {
> // Wait for the client to drop the connection
> try {
> clientCloseLatch.await();
> @@ -1469,8 +1469,8 @@ public class TestNonBlockingAPI extends
> TomcatBaseTest {
> // Ignore
> }
> sos.print("TEST");
> - stage++;
> - } else if (stage == 2) {
> + stage.incrementAndGet();
> + } else if (stage.get() == 2) {
> sos.flush();
> }
> } while (sos.isReady());
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>
