views:

160

answers:

2

I'm working on a thread-safe collection that uses Dictionary as a backing store.

In C# you can do the following:

  private IEnumerable<KeyValuePair<K, V>> Enumerate() {
     if (_synchronize) {
        lock (_locker) {
           foreach (var entry in _dict)
              yield return entry;
        }
     } else {
        foreach (var entry in _dict)
           yield return entry;
     }
  }

The only way I've found to do this in F# is using Monitor, e.g.:

    let enumerate() =
        if synchronize then
            seq {
                System.Threading.Monitor.Enter(locker)
                try for entry in dict -> entry
                finally System.Threading.Monitor.Exit(locker)
            }
        else seq { for entry in dict -> entry }

Can this be done using the lock function? Or, is there a better way to do this in general? I don't think returning a copy of the collection for iteration will work because I need absolute synchronization.

+3  A: 

I don't think that you'll be able to do the same thing with the lock function, since you would be trying to yield from within it. Having said that, this looks like a dangerous approach in either language, since it means that the lock can be held for an arbitrary amount of time (e.g. if one thread calls Enumerate() but doesn't enumerate all the way through the resulting IEnumerable<_>, then the lock will continue to be held).

It may make more sense to invert the logic, providing an iter method along the lines of:

let iter f =
  if synchronize then
    lock locker (fun () -> Seq.iter f dict)
  else
    Seq.iter f dict

This brings the iteration back under your control, ensuring that the sequence is fully iterated (assuming that f doesn't block, which seems like a necessary assumption in any case) and that the lock is released immediately thereafter.

EDIT

Here's an example of code that could hold the lock forever.

let cached = enumerate() |> Seq.cache
let firstFive = Seq.take 5 cached |> Seq.toList

We've taken the lock in order to start enumerating through the first 5 items. However, we haven't continued through the rest of the sequence, so the lock won't be released (maybe we would enumerate the rest of the way later based on user feedback or something, in which case the lock would finally be released).

In most cases, correctly written code will ensure that it disposes of the original enumerator, but there's no way to guarantee that in general. Therefore, your sequence expressions should be designed to be robust to only being enumerated part way. If you intend to require your callers to enumerate the collection all at once, then forcing them to pass you the function to apply to each element is better than returning a sequence which they can enumerate as they please.

kvb
Great suggestion. Thanks. I'm curious how a thread not completely enumerating the collection could hold on to the lock, unless you mean something like calling Thread.Sleep(a really long time) in a for loop. Otherwise, it seems it would break out of the loop, calling Dispose on the enumerator and in turn releasing the lock.
Daniel
@Daniel - I've added an example which doesn't release the lock. If the calling code always uses a for loop, you're probably safe, but there are other ways to use IEnumerables.
kvb
@kvb - Thanks for the additional explanation.
Daniel
+2  A: 

I agree with kvb that the code is suspicious and that you probably don't want to hold the lock. However, there is a way to write the locking in a more comfortable way using the use keyword. It's worth mentioning it, because it may be useful in other situations.

You can write a function that starts holding a lock and returns IDisposable, which releases the lock when it is disposed:

let makeLock locker =
  System.Threading.Monitor.Enter(locker) 
  { new System.IDisposable with  
      member x.Dispose() =
        System.Threading.Monitor.Exit(locker) }

Then you can write for example:

let enumerate() = seq {
  if synchronize then       
    use l0 = makeLock locker
    for entry in dict do
      yield entry       
  else 
    for entry in dict do
      yield entry }

This is essentially implementing C# like lock using the use keyword, which has similar properties (allows you to do something when leaving the scope). So, this is much closer to the original C# version of the code.

Tomas Petricek