From 7fdac0a5c2f23b12dae328837148e19295fad735 Mon Sep 17 00:00:00 2001
From: Adam Reichold <adam.reichold@t-online.de>
Date: Mon, 26 Jun 2023 19:05:39 +0200
Subject: [PATCH] Stop re-using NthIndexCache to avoid ABA problem

I just noticed that the NthIndexCache is internally keyed on the pointer to the
selectors themselves which I think opens this up for an ABA problem, i.e. after
dropping a selector and allocating a new one at the same address but with
different content, the cache might result in incorrect matches.

Hence, I just avoided re-use completely which is certainly correct if
unnecessarily inefficient, but I fear proper re-use would need to be
user-visible.
---
 src/selector.rs | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/src/selector.rs b/src/selector.rs
index 97750512..0e482ca9 100644
--- a/src/selector.rs
+++ b/src/selector.rs
@@ -1,6 +1,5 @@
 //! CSS selectors.
 
-use std::cell::RefCell;
 use std::convert::TryFrom;
 use std::fmt;
 
@@ -10,7 +9,6 @@ use html5ever::{LocalName, Namespace};
 use selectors::{
     matching,
     parser::{self, ParseRelative, SelectorParseErrorKind},
-    NthIndexCache,
 };
 
 use crate::error::SelectorErrorKind;
@@ -46,25 +44,19 @@ impl Selector {
     /// The optional `scope` argument is used to specify which element has `:scope` pseudo-class.
     /// When it is `None`, `:scope` will match the root element.
     pub fn matches_with_scope(&self, element: &ElementRef, scope: Option<ElementRef>) -> bool {
-        thread_local! {
-            static NTH_INDEX_CACHE: RefCell<NthIndexCache> = Default::default();
-        }
-
-        NTH_INDEX_CACHE.with(|nth_index_cache| {
-            let mut nth_index_cache = nth_index_cache.borrow_mut();
-            let mut context = matching::MatchingContext::new(
-                matching::MatchingMode::Normal,
-                None,
-                &mut nth_index_cache,
-                matching::QuirksMode::NoQuirks,
-                matching::NeedsSelectorFlags::No,
-                matching::IgnoreNthChildForInvalidation::No,
-            );
-            context.scope_element = scope.map(|x| selectors::Element::opaque(&x));
-            self.selectors
-                .iter()
-                .any(|s| matching::matches_selector(s, 0, None, element, &mut context))
-        })
+        let mut nth_index_cache = Default::default();
+        let mut context = matching::MatchingContext::new(
+            matching::MatchingMode::Normal,
+            None,
+            &mut nth_index_cache,
+            matching::QuirksMode::NoQuirks,
+            matching::NeedsSelectorFlags::No,
+            matching::IgnoreNthChildForInvalidation::No,
+        );
+        context.scope_element = scope.map(|x| selectors::Element::opaque(&x));
+        self.selectors
+            .iter()
+            .any(|s| matching::matches_selector(s, 0, None, element, &mut context))
     }
 }