From 96fa99ee6c8ad9b646ed872519da79971add98e4 Mon Sep 17 00:00:00 2001 From: Stefan Bodewig Date: Sat, 22 May 2021 13:29:28 +0200 Subject: [PATCH] create local copies of realThing to avoid TOCTOU race condition see https://bz.apache.org/bugzilla/show_bug.cgi?id=65316 --- WHATSNEW | 3 + .../org/apache/tools/ant/UnknownElement.java | 55 +++++++++++-------- 2 files changed, 35 insertions(+), 23 deletions(-) diff --git a/WHATSNEW b/WHATSNEW index d270d902aa..1ff6c1c29a 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -4,6 +4,9 @@ Changes from Ant 1.10.10 TO Ant 1.10.11 Fixed bugs: ----------- + * a race condition could lead to NullPointerExceptions when running + tasks in parallel. + Bugzilla Report 65316 Other changes: -------------- diff --git a/src/main/org/apache/tools/ant/UnknownElement.java b/src/main/org/apache/tools/ant/UnknownElement.java index f33dd525bc..fce44660c1 100644 --- a/src/main/org/apache/tools/ant/UnknownElement.java +++ b/src/main/org/apache/tools/ant/UnknownElement.java @@ -159,7 +159,8 @@ public RuntimeConfigurable getWrapper() { * @exception BuildException if the configuration fails */ public void maybeConfigure() throws BuildException { - if (realThing != null) { + final Object copy = realThing; + if (copy != null) { return; } configure(makeObject(this, getWrapper())); @@ -177,10 +178,10 @@ public void configure(Object realObject) { } realThing = realObject; - getWrapper().setProxy(realThing); + getWrapper().setProxy(realObject); Task task = null; - if (realThing instanceof Task) { - task = (Task) realThing; + if (realObject instanceof Task) { + task = (Task) realObject; task.setRuntimeConfigurableWrapper(getWrapper()); @@ -188,7 +189,7 @@ public void configure(Object realObject) { // targets to work. *very* Ugly // The reference is replaced by RuntimeConfigurable if (getWrapper().getId() != null) { - this.getOwningTarget().replaceChild(this, (Task) realThing); + this.getOwningTarget().replaceChild(this, (Task) realObject); } } @@ -203,7 +204,7 @@ public void configure(Object realObject) { getWrapper().maybeConfigure(getProject()); } - handleChildren(realThing, getWrapper()); + handleChildren(realObject, getWrapper()); } /** @@ -212,8 +213,9 @@ public void configure(Object realObject) { * @param output The output to log. Should not be null. */ protected void handleOutput(String output) { - if (realThing instanceof Task) { - ((Task) realThing).handleOutput(output); + final Object copy = realThing; + if (copy instanceof Task) { + ((Task) copy).handleOutput(output); } else { super.handleOutput(output); } @@ -233,8 +235,9 @@ protected void handleOutput(String output) { */ protected int handleInput(byte[] buffer, int offset, int length) throws IOException { - if (realThing instanceof Task) { - return ((Task) realThing).handleInput(buffer, offset, length); + final Object copy = realThing; + if (copy instanceof Task) { + return ((Task) copy).handleInput(buffer, offset, length); } return super.handleInput(buffer, offset, length); } @@ -245,8 +248,9 @@ protected int handleInput(byte[] buffer, int offset, int length) * @param output The output to log. Should not be null. */ protected void handleFlush(String output) { - if (realThing instanceof Task) { - ((Task) realThing).handleFlush(output); + final Object copy = realThing; + if (copy instanceof Task) { + ((Task) copy).handleFlush(output); } else { super.handleFlush(output); } @@ -258,8 +262,9 @@ protected void handleFlush(String output) { * @param output The error output to log. Should not be null. */ protected void handleErrorOutput(String output) { - if (realThing instanceof Task) { - ((Task) realThing).handleErrorOutput(output); + final Object copy = realThing; + if (copy instanceof Task) { + ((Task) copy).handleErrorOutput(output); } else { super.handleErrorOutput(output); } @@ -271,8 +276,9 @@ protected void handleErrorOutput(String output) { * @param output The error output to log. Should not be null. */ protected void handleErrorFlush(String output) { - if (realThing instanceof Task) { - ((Task) realThing).handleErrorFlush(output); + final Object copy = realThing; + if (copy instanceof Task) { + ((Task) copy).handleErrorFlush(output); } else { super.handleErrorFlush(output); } @@ -283,13 +289,14 @@ protected void handleErrorFlush(String output) { * (e.g. a data type) then this method does nothing. */ public void execute() { - if (realThing == null) { + final Object copy = realThing; + if (copy == null) { // Got here if the runtimeconfigurable is not enabled. return; } try { - if (realThing instanceof Task) { - ((Task) realThing).execute(); + if (copy instanceof Task) { + ((Task) copy).execute(); } } finally { // Finished executing the task @@ -503,8 +510,9 @@ protected BuildException getNotFoundException(String what, * @return the name to use in logging messages. */ public String getTaskName() { - return !(realThing instanceof Task) ? super.getTaskName() - : ((Task) realThing).getTaskName(); + final Object copy = realThing; + return !(copy instanceof Task) ? super.getTaskName() + : ((Task) copy).getTaskName(); } /** @@ -514,8 +522,9 @@ public String getTaskName() { * a task. */ public Task getTask() { - if (realThing instanceof Task) { - return (Task) realThing; + final Object copy = realThing; + if (copy instanceof Task) { + return (Task) copy; } return null; }