5

I have this method:

public bool Remove(EntityKeyType key) { lock (syncroot) { //wait if we need to waitForContextMRE.Wait(); //if the item is not local, assume it is not remote. if (!localCache.ContainsKey(key)) return false; //build an expression tree Expression<Func<EntityType, bool>> keyComparitorExpression = GenerateKeyComparitorExpression(key); var itemToDelete = TableProperty.Single(keyComparitorExpression); //delete from db TableProperty.DeleteOnSubmit(itemToDelete); DataContext.SubmitChanges(); //get the removed item for OnCollectionChanged EntityType itemToRemove = localCache[key]; itemToRemove.PropertyChanged -= item_PropertyChanged; //remove from the list Debug.Assert(localCache.Remove(key)); //call the notification OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, itemToRemove)); return true; } }

I am calling it from multiple threads (calling the same instance), but an exception keeps being thrown on TableProperty.Single (Sequence contains no elements). Upon debugging the code I saw that a situation is being created where the item is being deleted from the database after a different thread has checked the cache for its existence. This should not be possible unless there are multiple threads inside the lock statement (The syncroot object is definitely the same instance across threads).

Impossible? I have proof: Impossible situation

There are three threads inside the lock statement! What gives?

notes:

  1. The MRE is set (not blocking).
  2. This is not a situation where the exception gets thrown, it just shows multiple threads inside a lock section. Update: I changed the image to an intellitrace event of the exception. The old image is here
  3. The syncroot object is not static, because I only want calls to the same instance syncronized.

Update

This is the declaration of the syncroot object:

private object syncroot = new object();

And some other declarations:

private ManualResetEventSlim waitForContextMRE = new ManualResetEventSlim(true); private DataContextType _dataContext; private System.Data.Linq.Table<EntityType> _tableProperty; //DataContextType and EntityType are generic type parameters

I cannot make the syncroot static because I have several instances of the class running and it is important that they don't block each other. But that doesn't really matter - making it static does not fix the problem.

The ManualResetEvent (waitForContextMRE) is not there for synchronization - it's there to block database operations for a certain time after certain operations are performed (i.e. on startup). It is set most of the time. Taking it out of the lock block also does not fix the problem.

Improve this question
11
  • 1
    A: can we see where you are initialising syncroot, and B: how long is the object-context kicking around for? C: are you sure they are the same instance? Hard to tell from a paused state... Commented Dec 25, 2011 at 22:10
  • 1
    I would suggest adding trace messages, with System.Diagnostics.Debug.WriteLine. Write the thread id and the syncroot hashcode at the beginning and the end of the lock block. Then you can easily see in the output window if two threads overlap with the same syncroot, which definitely shouldn't happen. Commented Dec 25, 2011 at 22:22
  • What's the harm in making syncroot static? Commented Dec 25, 2011 at 22:24
  • 1
    @Bob presumably it is intended to be more granular than one-per-app; in many apps a static lock is very much to be avoided Commented Dec 25, 2011 at 22:26
  • 1
    @Bob absoltely; assuming this is a high usage area, it renders your multi-core server fairly useless - instead allowing just a single caller. If the data is (as, it seems, the OP intends) separate, there is no need for them to block each-other. If it was just local access it would be undesirable but forgivable (at least until profiling shows it to be important) - but when adding network access (DB in this case) into the mix, it can represent a significant pinch-point. Commented Dec 25, 2011 at 23:53

4 Answers 4

Reset to default
3

The only explanation I have is that waitForContextMRE.Wait(); calling this makes the thread do unblock syncroot! And so other thread can enter lock section. Try to move waitForContextMRE.Wait(); before lock(...).

Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I don't think we can see enough about waitForContextMRE to make any conclusions there; Monitor.Wait(syncroot) is the only thing that would release the lock - and it isn't obvious to me that this is the same
Why call wait() if you are inside the lock and assuming there is no other process using the thread? //wait if we need to waitForContextMRE.Wait();
3

I think that you are calling different objects. There is no indication on your screenshot that you are taking values from different threads. Also using non static syncroot is not a good idea because may result in cases like yours. Do you have really strong reason not to have it static?

Improve this answer

2 Comments

@Elastep I concur that the evidence of being the same object is fairly lacking - hence my request for more information as an (unanswered) comment...
It needs to be nonstatic because there will be multiple instances of the object that must not block each other. This code is being called from a unit test and in that case there is just one instance.
0

I suggest lock TableProperty or DataContext

Improve this answer

2 Comments

Out of curiosity... based on? Threading issues are so complex that any knee-jerk reaction should be avoided, and any recommendation should be a: based on evidence, and b: include reasoning
I cannot lock the TableProperty or DataContext because there are situations where they are changed (disposed and re-created) and in that case it also locks the syncroot object.
0

I have been debugging this problem for a while now and, although I have not resolved it, it is clear to me that the locks are working. I am guessing the problem is with the DataContext (which are known for being tricky in multithreaded situations).

Improve this answer

1 Comment

I have found the problem - Debug.Assert(localCache.Remove(key)) - Just like in C++ the stuff inside the assert does not run in release mode, causing items to be deleted from the database and not from the cache.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.