Is this combination of ConcurrentDictionary and ConcurrentQueue thread safe?

Question

I use the ConcurrentDictionary and ConcurrentQueue classes from .NET 4 in the following code.

Is this code thread safe? If not, how can I make it thread safe?

public class Page { public string Name {get; set; } } public class PageQueue { private ConcurrentDictionary<int, ConcurrentQueue<Page>> pages = new ConcurrentDictionary<int, ConcurrentQueue<Page>>(); public void Add(int id, Page page) { if (!this.pages.ContainsKey(id)) this.pages[id] = new ConcurrentQueue<Page>(); this.pages[id].Enqueue(page); } public Page GetAndRemove(int id) { Page lp = null; if(this.pages.ContainsKey(id)) this.pages[id].TryDequeue(out lp); return lp; } } 

Demo:

  public class Demo { public void RunAll() { for (int i = 0; i < 10; i++) Task.Factory.StartNew(() => Run()); } public void Run() { PageQueue pq = new PageQueue(); pq.Add(1, new Page()); pq.GetAndRemove(1); } } 

Answer 1

As @Femaref correctly pointed out, there are some flaws in your code. I suggest you take advantage of the many methods suggested by ConcurrentDictionary <K, V> to make the code safe without having to use a lock statement:

 public class PageQueue { private ConcurrentDictionary<int, ConcurrentQueue<Page>> pages = new ConcurrentDictionary<int, ConcurrentQueue<Page>>(); public void Enqueue(int id, Page page) { var queue = this.pages.GetOrAdd(id, _ => new ConcurrentQueue<Page>()); queue.Enqueue(page); } public bool TryDequeue(int id, out Page page) { ConcurrentQueue<Page> queue; if (this.pages.TryGetValue(id, out queue)) { return queue.TryDequeue(out page); } page = null; return false; } } 

Answer 2

You can (and probably will) work with problems with these statements:

  if (!this.pages.ContainsKey(id)) this.pages[id] = new ConcurrentQueue<Page>(); 

and

  if(this.pages.ContainsKey(id)) this.pages[id].TryDequeue(out lp); 

since ConcurrentDictionary can be changed between if-statement and Assignment / Dequeue. Use a lock on the lock object for these pieces of code, for example:

 public class PageQueue { private ConcurrentDictionary<int, ConcurrentQueue<Page>> pages = new ConcurrentDictionary<int, ConcurrentQueue<Page>>(); private object locker = new object(); public void Add(int id , Page page) { lock(locker) { if (!this.pages.ContainsKey(id)) this.pages[id] = new ConcurrentQueue<Page>(); } this.pages[id].Enqueue(page); } public Page GetAndRemove(int id) { Page lp = null; lock(locker) { if(this.pages.ContainsKey(id)) this.pages[id].TryDequeue(out lp); } return lp; } }