Skip to content

Commit

Permalink
AbstractDomNodeList access might throw a NPE due to a race condition …
Browse files Browse the repository at this point in the history
…in the DomHtmlAttributeChangeListenerImpl clearing of cachedElements processing (#882)
  • Loading branch information
rbri committed Oct 16, 2024
1 parent 822662c commit bd81428
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/changes/changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
</action>
<action type="fix" dev="rbri" issue="#882">
AbstractDomNodeList access might throw a NPE due to a race condition in the DomHtmlAttributeChangeListenerImpl
clearing of cachedElements processing.
</action>
<action type="fix" dev="rbri">
Function initHashChangeEvent() is no longer available in FF_ESR.
</action>
Expand Down
20 changes: 15 additions & 5 deletions src/main/java/org/htmlunit/html/AbstractDomNodeList.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ public abstract class AbstractDomNodeList<E extends DomNode> 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) {
Expand Down Expand Up @@ -81,10 +81,20 @@ protected DomNode getDomNode() {
* @return the nodes in this node list
*/
private List<E> 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:/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<E> providedElements = provideElements();
cachedElements_ = providedElements;
return providedElements;
}

/**
Expand Down

0 comments on commit bd81428

Please sign in to comment.