From bd814289ea71ec502776b2b964c234969200f9b7 Mon Sep 17 00:00:00 2001 From: Ronald Brill Date: Wed, 16 Oct 2024 07:11:31 +0200 Subject: [PATCH] AbstractDomNodeList access might throw a NPE due to a race condition in the DomHtmlAttributeChangeListenerImpl clearing of cachedElements processing (#882) --- src/changes/changes.xml | 4 ++++ .../htmlunit/html/AbstractDomNodeList.java | 20 ++++++++++++++----- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 18b2a9ccb0..4fd38a62df 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -12,6 +12,10 @@ Special handling of the bgsound tag in FF removed. BGSound is now handled as unknown tag in all browsers. Looks like this was a leftover from the IE days. + + AbstractDomNodeList access might throw a NPE due to a race condition in the DomHtmlAttributeChangeListenerImpl + clearing of cachedElements processing. + Function initHashChangeEvent() is no longer available in FF_ESR. diff --git a/src/main/java/org/htmlunit/html/AbstractDomNodeList.java b/src/main/java/org/htmlunit/html/AbstractDomNodeList.java index fffbe43fc8..5f1cafdefa 100644 --- a/src/main/java/org/htmlunit/html/AbstractDomNodeList.java +++ b/src/main/java/org/htmlunit/html/AbstractDomNodeList.java @@ -47,13 +47,13 @@ public abstract class AbstractDomNodeList extends AbstractLis */ public AbstractDomNodeList(final DomNode node) { super(); + node_ = node; + if (node == null) { - node_ = null; cachedElements_ = Collections.EMPTY_LIST; return; } - node_ = node; final DomHtmlAttributeChangeListenerImpl listener = new DomHtmlAttributeChangeListenerImpl(this); node_.addDomChangeListener(listener); if (node_ instanceof HtmlElement) { @@ -81,10 +81,20 @@ protected DomNode getDomNode() { * @return the nodes in this node list */ private List getNodes() { - if (cachedElements_ == null) { - cachedElements_ = provideElements(); + if (cachedElements_ != null) { + return cachedElements_; } - return cachedElements_; + + // a bit of a hack but i like to avoid synchronization + // see https://github.com/HtmlUnit/htmlunit/issues/882 + // + // there is a small chance that the cachedElements_ are + // set to null after the assignment and before the return + // but this is a race condition at all and depending on the + // thread state the same overall result might happen also with sync + final List providedElements = provideElements(); + cachedElements_ = providedElements; + return providedElements; } /**