I believe fileChangeService is now truely free-thread?

It is, but we could have multiple callers of this at the same time if we have multiple projects trying to use the same file. We call this outside of the lock that's guarding the dictionary of all of these.

reason it being internal?

Ah, right, that was from before I did the IReferenceCountedDisposable trick.

is this guranteed to be called after InitializeFileTracking?

InitializeFileTracking is called before we ever hand out a rule set file, so yes.

if this file is tracked by IReferenceCountDispose, is it okay to call Dispose directly like this without going through IReferenceCountDipose contract?

As long as Dispose is idempotent, it's fine, which it's normally supposed to be anyways. I guess there's a small bug here because we might end up doing StopTrackingRuleSetFile more than once in that case. Will fix.

Can this throw Argument Exception “Item with Same Key has already been added” for case 2. above?

Fixed.

💭 _disposed is an anti-pattern for types wrapped in ReferenceCountedDisposable<T>, since it provides both a single-dispose guarantee and a way for callers to ensure the type is never used after dispose in a concurrent environment.

Renamed this and extracted Dispose to a helper for clarity.

❗️ This breaks an invariant of ReferenceCountedDisposable<T>: If you obtain a non-null reference using TryGetReference and have not called Dispose(), then the target object is not disposed. This is an essential invariant for reasoning about the type at scale.

💡 internal so it's clear on review that it can't be part of IRuleSetFile

Won't fix: HeeJae asked why it was internal. At this point not going to play flip flop.

💭 I'm concerned about this being called inside the lock, since it's generally not good to assume the call will execute asynchronously.

The API is actually void returning -- we call it Async to indicate it doesn't do synchronously. I expect some larger file change watching rewrite to happen in a month or two (or part of the bigger change) so this might get a lot better.

Incidently this wasn't a new problem for this code: if you see the handler for the file change, you'll see it was doing a workarould already. The lock was just invisible: the UI thead was the implicit lock.

💭 This is another call inside a lock that makes me uncomfortable

Thinking about it again, I think we're good. In some sense this problem already existed; it was being called in the context of the lock known as the UI thread.

💡 Consider using ImmutableArray<FileChangeTracker> or ImmutableList<FileChangeTracker> with atomic exchange. The former offers better efficiency if the list is typically constructed from empty, while the latter offers efficiency if the list is typically incrementally populated.

We directly create the type with the known capacity, so that'll be fastest of all, right?

Ah, this wasn't a 💡 for efficiency. The immutable collections allow some flexibility with respect to hoisting cleanup operations outside the locks (you can exchange the list with an empty list, then use the local copy to clean things up).

Aiming for KISS here.

💡 Consider one of the following:

1. Use the following code to check instances

   if (_ruleSetFileMap.TryGetValue(ruleSetFile, out var weakReference)) {
     using (var reference = weakReference.TryGetReference()) {
       if (reference == null || reference.Target == ruleSetFile) {
         _ruleSetFileMap.Remove(ruleSetFile.FilePath);
       }
     }
   }

2. Modify the comment to instead explain that clearing the cache for a different RuleSetFile is not problematic (correctness, plus the performance cost of recreating the item amortized against the likelihood that a mismatch occurs)

I wanted to avoid 1 to keep it simple. Comment was intended to do 2 but I'll clarify further.

❕ If possible without breaking the tests, replace Path.GetTempPath() with TempPath.Root. This ensures the temporary directory doesn't get littered even if cleanup fails.

@sharwell You mean something else? my IDE can't find TempPath.Root.

Sorry, it's TempRoot.Root.