Thread safe for Path.toRelativeFrom

[humhei]

Description

I encounted this in async workflow
Null Reference expection was throw

before (dictionary is global)

let toRelativeFrom =
    /// A cache of relative path names.
    /// [omit]
    let relativePaths = new Dictionary<_, _>()

    /// Replaces the absolute path to a relative path.
    let inline toRelativePath basePath value =
        let key = (basePath, value)
        match relativePaths.TryGetValue key with
        | true, x -> x
        | _ -> 
            let x = ProduceRelativePath basePath value
            relativePaths.Add(key, x)
            x

    toRelativePath

now (dictionary is in block)

let toRelativeFrom basePath value =
    /// A cache of relative path names.
    /// [omit]
    let relativePaths = new Dictionary<_, _>()

    /// Replaces the absolute path to a relative path.
    let inline toRelativePath basePath value =
        let key = (basePath, value)
        match relativePaths.TryGetValue key with
        | true, x -> x
        | _ -> 
            let x = ProduceRelativePath basePath value
            relativePaths.Add(key, x)
            x

    toRelativePath basePath value

[csmager]

While this fixes the issue of concurrent access, it defeats the purpose of the cache - you might as well replace all the code with ProduceRelativePath basePath value.

If we want to cache the results and make this thread safe you'd need to add locking or use something like ConcurrentDictionary instead.

[matthid]

Agreed with @csmager. Either we remove the cache completely (and live with potential performance regressions) or we use some kind of locking or ConcurrentDictionary

[matthid]

Thanks for taking care of this. A test would have been nice but I can see that this is difficult to test.