Skip to content

Commit 3d34177

Browse files
committed
Invalidate mixin cache on file change to prevent stale members
1 parent 3db99e7 commit 3d34177

4 files changed

Lines changed: 42 additions & 17 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5959

6060
### Fixed
6161

62+
- **Stale mixin members after editing.** Mixin class resolution (e.g. `@mixin Builder`) is now invalidated when any file changes, so newly added or removed methods on mixin targets appear immediately without restarting the server.
6263
- **Version-gated stub constants now filtered.** Constants with `@removed` tags (e.g. `MCRYPT_ENCRYPT`, removed in PHP 7.2) are now excluded from completion and resolution when the project targets a newer PHP version. Previously only classes and functions were filtered.
6364
- **Go-to-definition.** Fixed a potential deadlock when navigating to a vendor class that hadn't been parsed yet.
6465
- **LSP no longer freezes under heavy editor activity.** Server-to-client requests (diagnostic refresh, progress token creation) could deadlock the service loop when the editor was simultaneously sending bursts of open/close/hover messages. All server-to-client requests are now either fire-and-forget or time-bounded, long-running handlers are cancellation-safe, and the process exits cleanly if the service loop ever terminates unexpectedly.

docs/todo/refactor.md

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -258,15 +258,6 @@ only during diagnostic/analyse passes. Completion and hover re-resolve callable
258258
targets and re-parse files from scratch. Extending these to all consumers would
259259
avoid redundant work.
260260

261-
### `MIXIN_CACHE` never invalidated
262-
263-
`src/virtual_members/phpdoc.rs` `MIXIN_CACHE` is a thread-local
264-
`HashMap<String, Arc<ClassInfo>>` that caches resolved mixin classes. It is used
265-
by all consumers (completion, hover, diagnostics) but is never cleared during
266-
normal LSP operation. If a mixin class definition changes (e.g. a method is added
267-
to `Illuminate\Database\Eloquent\Builder`), the cache serves stale data until the
268-
thread dies. Needs content-hash or generation-counter invalidation.
269-
270261
---
271262

272263
## Redundant backwards text walkers

src/parser/ast_update.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ impl Backend {
3333
/// changed (or a class was added/removed), meaning other open files
3434
/// that reference those classes may have stale diagnostics.
3535
pub fn update_ast(&self, uri: &str, content: &str) -> bool {
36+
// Invalidate thread-local mixin cache so stale ClassInfo is not
37+
// served after a file changes.
38+
crate::virtual_members::phpdoc::bump_mixin_generation();
39+
3640
let content_to_parse = if self.is_blade_file(uri) {
3741
let (virtual_php, source_map) = crate::blade::preprocessor::preprocess(content);
3842
self.blade_source_maps

src/virtual_members/phpdoc.rs

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
1919
use std::cell::RefCell;
2020
use std::collections::{HashMap, HashSet};
21+
use std::sync::atomic::{AtomicU64, Ordering};
2122
use std::sync::Arc;
2223

2324
use crate::atom::{Atom, atom};
@@ -31,6 +32,17 @@ use crate::types::{
3132
};
3233
use crate::util::short_name;
3334

35+
/// Global generation counter, incremented every time a file is re-parsed.
36+
/// Thread-local caches compare against this to detect staleness.
37+
static MIXIN_GENERATION: AtomicU64 = AtomicU64::new(0);
38+
39+
/// Bump the mixin-cache generation so that all threads discard stale entries
40+
/// on their next access. Called from [`Backend::update_ast`] whenever a file
41+
/// changes.
42+
pub fn bump_mixin_generation() {
43+
MIXIN_GENERATION.fetch_add(1, Ordering::Relaxed);
44+
}
45+
3446
thread_local! {
3547
/// Thread-local cache of base-resolved mixin classes.
3648
///
@@ -40,11 +52,10 @@ thread_local! {
4052
/// `\Illuminate\Database\Eloquent\Builder`) are performed at most
4153
/// once per thread.
4254
///
43-
/// Must be cleared between test runs via [`clear_mixin_cache`]
44-
/// because different tests may define classes with the same short
45-
/// name but different members.
46-
static MIXIN_CACHE: RefCell<HashMap<String, Arc<ClassInfo>>> =
47-
RefCell::new(HashMap::new());
55+
/// Automatically invalidated when the global generation counter
56+
/// advances (i.e. when any file is re-parsed).
57+
static MIXIN_CACHE: RefCell<(u64, HashMap<String, Arc<ClassInfo>>)> =
58+
RefCell::new((0, HashMap::new()));
4859
}
4960

5061
/// Clear the thread-local mixin resolution cache.
@@ -55,7 +66,24 @@ thread_local! {
5566
/// different members. Call this function when creating a new test
5667
/// backend so that stale entries from a previous test do not leak.
5768
pub fn clear_mixin_cache() {
58-
MIXIN_CACHE.with(|cache| cache.borrow_mut().clear());
69+
MIXIN_CACHE.with(|cache| {
70+
let mut inner = cache.borrow_mut();
71+
inner.0 = MIXIN_GENERATION.load(Ordering::Relaxed);
72+
inner.1.clear();
73+
});
74+
}
75+
76+
/// Ensure the thread-local cache is current with the global generation.
77+
/// Clears the cache if stale.
78+
fn ensure_mixin_cache_fresh() {
79+
MIXIN_CACHE.with(|cache| {
80+
let current_gen = MIXIN_GENERATION.load(Ordering::Relaxed);
81+
let mut inner = cache.borrow_mut();
82+
if inner.0 != current_gen {
83+
inner.0 = current_gen;
84+
inner.1.clear();
85+
}
86+
});
5987
}
6088

6189
/// Tracks member names already seen during mixin collection.
@@ -603,6 +631,7 @@ fn collect_mixin_members(
603631
//
604632
// Results are cached in a thread-local map so that the same
605633
// mixin (e.g. Builder) is only resolved once per thread.
634+
ensure_mixin_cache_fresh();
606635
let resolved_mixin = if let Some(c) = cache {
607636
let map = c.lock();
608637
if let Some(cached) = map.get(&(Atom::from(&resolved_mixin_name), Vec::new())) {
@@ -611,7 +640,7 @@ fn collect_mixin_members(
611640
drop(map);
612641
MIXIN_CACHE.with(|thread_cache| {
613642
let mut map = thread_cache.borrow_mut();
614-
Arc::clone(map.entry(resolved_mixin_name.clone()).or_insert_with(|| {
643+
Arc::clone(map.1.entry(resolved_mixin_name.clone()).or_insert_with(|| {
615644
Arc::new(crate::inheritance::resolve_class_with_inheritance(
616645
&mixin_class,
617646
class_loader,
@@ -622,7 +651,7 @@ fn collect_mixin_members(
622651
} else {
623652
MIXIN_CACHE.with(|thread_cache| {
624653
let mut map = thread_cache.borrow_mut();
625-
Arc::clone(map.entry(resolved_mixin_name.clone()).or_insert_with(|| {
654+
Arc::clone(map.1.entry(resolved_mixin_name.clone()).or_insert_with(|| {
626655
Arc::new(crate::inheritance::resolve_class_with_inheritance(
627656
&mixin_class,
628657
class_loader,

0 commit comments

Comments
 (0)