All threads only in one method at a time?

Question

I have several objects inheriting from ClassA, which has an abstract method MethodA.

Each of these inheriting objects can allow up to a specific number of threads simutaneously into their MethodA.

The catch: Threads can only be in an object's MethodA, while no other objects' MethodA is being processed at the same time.

How can I solve this? I am thinking about using a semaphore, but don't know exactly how to do it, because I just can't wrap my head around the problem enough to get a solution.

EDIT:

Example code (may contain syntax errors:)

public class ClassA
{
  public virtual void MethodA{}
}

public class OneOfMySubclassesOfClassA // there can be multiple instances of each subclass!
{
  public override void MethodA{

  // WHILE any number of threads are in here, THIS MethodA shall be the ONLY MethodA in the entire program to contain threads
  EDIT2: // I mean: ...ONLY MethodA of a subclass (not of a instance of a subclass) in the entire program...
}
}

...and more subclasses...

I suggest you post some code that will let people assist you. This really is a very vague question. — Daniel Kelley, Jan 11 '13 at 14:49
if I get you correctly, you want to fire multiple threads doing the work of MethodA, but only once the previous thread has finished? — CuccoChaser, Jan 11 '13 at 14:50
May look at the semaphore class: http://msdn.microsoft.com/en-us/library/system.threading.semaphore.aspx — CodeTherapist, Jan 11 '13 at 14:51
@FlorisPrijt my understanding is he wants to allow a group of X threads into MethodA but only one group of X threads at a time — , Jan 11 '13 at 14:52
if you want to control some aspect of MethodA, you should not have it as an abstract method — Dhawalk, Jan 11 '13 at 14:53
Is there only one instance of each subclass shared by all threads or is it possible that several threads are executing the method of different instance of the same subclass? — Daniel Brückner, Jan 11 '13 at 14:53
I'll rephase my post: I want threads only in 1 methodA (methodA is inherited by multiple objects) at any given time. — cdbeelala89, Jan 11 '13 at 14:57
There are several methods `MethodA` on different derived types - `ClassA1.MethodA`, `ClassA2.MethodA`, `ClassA3.MethodA` and so on. At any point in time threads are only allowed to execute one of this methods. Four threads in method `ClassA2.MethodA` is valid, but not one thread executing `ClassA1.MethodA` and three threads executing `ClassA2.MethodA`. — Daniel Brückner, Jan 11 '13 at 14:57
@ Daniel Brückner that is correct. I put it shorter in the reponse above your comment. so it seems I need a semaphore? or multiple semaphore? — cdbeelala89, Jan 11 '13 at 14:58
The exact structure depends on the number of instances - one instance per thread, one instance shared by all threads, a mix of that, ... — Daniel Brückner, Jan 11 '13 at 15:01
@ Daniel Brückner ***EDIT***: is IS possible that several threads are executing the method of different instances of the same subclass (fyi: the subclasses are asp controllers inheriting from a base controller class. And I believe asp creates an instance of a controller for each http request it recieves, so yeah..) @Dhawalk does not need to be abstract method necessarily. — cdbeelala89, Jan 11 '13 at 15:02
This is a very strange and therefore probably artificial request. Locking is needed around data, not around methods. So the core of this question makes little sense. — H H, Jan 11 '13 at 15:31

Daniel Brückner · Accepted Answer · 2013-01-11T16:58:52.943

The derived type is used as type argument in the base class together with a static semaphore to get one semaphore shared between all instances of each subclass. And then there is some mess to ensure that only one type is active. A quick test indicates that this works correctly but there is an issue.

Assume for example that the method of ClassA1 is currently executing. If new request to execute this methods arrive with high frequency it may happen that other derived classes get no chance to execute because there are constantly new threads executing the method of class ClassA1.

internal abstract class ClassA<TDerived> : ClassA
{
    private const Int32 MaximumNumberConcurrentThreads = 3;

    private static readonly Semaphore semaphore = new Semaphore(ClassA<TDerived>.MaximumNumberConcurrentThreads, ClassA<TDerived>.MaximumNumberConcurrentThreads);

    internal void MethodA()
    {
        lock (ClassA.setCurrentlyExcutingTypeLock)
        {
            while (!((ClassA.currentlyExcutingType == null) || (ClassA.currentlyExcutingType == typeof(TDerived))))
            {
                Monitor.Wait(ClassA.setCurrentlyExcutingTypeLock);
            }

            if (ClassA.currentlyExcutingType == null)
            {
                ClassA.currentlyExcutingType = typeof(TDerived);
            }

            ClassA.numberCurrentlyPossiblyExecutingThreads++;

            Monitor.PulseAll(ClassA.setCurrentlyExcutingTypeLock);
        }

        try
        {
            ClassA<TDerived>.semaphore.WaitOne();

            this.MethodACore();
        }
        finally
        {
            ClassA<TDerived>.semaphore.Release();
        }

        lock (ClassA.setCurrentlyExcutingTypeLock)
        {
            ClassA.numberCurrentlyPossiblyExecutingThreads--;

            if (ClassA.numberCurrentlyPossiblyExecutingThreads == 0)
            {
                ClassA.currentlyExcutingType = null;

                Monitor.Pulse(ClassA.setCurrentlyExcutingTypeLock);
            }
        }
    }

    protected abstract void MethodACore();
}

Note that a wrapper method is used to call the actual implementation in MethodACore. All the synchronization objects shared between all derived classes are in a non-generic base class.

internal abstract class ClassA
{
    protected static Type currentlyExcutingType = null;

    protected static readonly Object setCurrentlyExcutingTypeLock = new Object();

    protected static Int32 numberCurrentlyPossiblyExecutingThreads = 0;
}

The derived classes will look like this.

internal sealed class ClassA1 : ClassA<ClassA1>
{
    protected override void MethodACore()
    {
        // Do work here.
    }
}

internal sealed class ClassA2 : ClassA<ClassA2>
{
    protected override void MethodACore()
    {
        // Do work here.
    }
}

Unfortunately I have no time to explain how and why this works in more detail right now but I will update the answer tomorrow.

looks very promising looking forward for the whole thing :) (need the part where it checks if the semaphores of the other subclasses are empty so it know whether it can do waitone() at all) — cdbeelala89, Jan 11 '13 at 15:33
Thank you very much for the full version, I bet that took some work. Just 2 questions though. Shouldn't the derived classes call MethodA instead of MethodACore? And shouldn't MethodA be virtual? Thanks again, I'll implement it when I get home tonight. — cdbeelala89, Jan 11 '13 at 16:54
Yes, that took some time. :D `ClassA.MethodA` does the heavy locking stuff and calls the abstract (and therefore implicitly virtual) method `ClassA.MethodACore` to do the actual work. This method is in turn overridden in the derived classes to implement the specific task for each derived class. — Daniel Brückner, Jan 11 '13 at 17:06
Keep in mind that this can be easily broken. Imagine `MeanClass : ClassA`. There's no restriction that the type that's in the generic parameter is the type of the instance itself. — Servy, Jan 11 '13 at 17:54
That's true, of course. This whole mess is far from perfect. I really did not expect that this questions turns out this way. I strongly believe there is a far better solution than mine. — Daniel Brückner, Jan 11 '13 at 18:14
OK, I'm here to report everything works fine with high frequency http requests sent to my controllers. I needed this mechanism because of a client unit test of mine that deletes all entites on the server that were previously submitted to the client, on entity by entity basis (highly multithreaded too). since my server uses nhibernate and has cascade.delete references, the transactions of the high frequency deletes were sometimes conflicting if entity types were deleted in random sequences while the deletes were also overlapping. o well, maybe somebody will understand :) nevermind :) — cdbeelala89, Jan 11 '13 at 18:41

Servy · Answer 2 · 2013-01-11T18:12:39.200

1

public abstract class Foo
{
    private static Type lockedType = null;
    private static object key = new object();
    private static ManualResetEvent signal = new ManualResetEvent(false);
    private static int threadsInMethodA = 0;
    private static Semaphore semaphore = new Semaphore(5, 5);//TODO set appropriate number of instances

    public void MethodA()
    {
        lock (key)
        {
            while (lockedType != this.GetType())
            {
                if (lockedType == null)
                {
                    lockedType = this.GetType();
                    //there may be other threads trying to get into the instance we just locked in
                    signal.Set();
                }
                else if (lockedType != this.GetType())
                {
                    signal.WaitOne();
                }
            }

            Interlocked.Increment(ref threadsInMethodA);
        }
        semaphore.WaitOne();

        try
        {
            MethodAImplementation();
        }
        finally
        {
            lock (key)
            {
                semaphore.Release();
                int threads = Interlocked.Decrement(ref threadsInMethodA);
                if (threads == 0)
                {
                    lockedType = null;
                    signal.Reset();
                }
            }
        }
    }

    protected abstract void MethodAImplementation();
}

So there are a few key points here. First off, we have a static object that represents the only instance that is allowed to have threads. null means the next thread to come along can put in "their" instance. If another instance is the "active" one the current thread waits on the manual reset event until either there is no locked instance, or the locked instance changed to what might possibly be that thread's instance.

It's also important to count the number of threads in the method to know when to set the locked instance to null (setting to to null without keeping track of that would let new instances start while a few of the previous instances were finishing.

Locks around another key at the start and end are rather important.

Also beware that with this setup it's possible for one type to starve out other types, so if this is a heavily contended resource it's something to watch out for.

edited Jan 11 '13 at 18:12

answered Jan 11 '13 at 15:30

Servy

202,030
26
332
449

Shouldn't the entire thing be in a try/finally to unsure proper incrementing and decrementing of `threadsInMethodA` – JG in SD Jan 11 '13 at 15:34
Cleaner than my solution: +1 – Henrik Jan 11 '13 at 15:46
This implementation does not allow concurrent execution of different instances of the same derived type. It does also not limit the maximum number of concurrently executing threads. – Daniel Brückner Jan 11 '13 at 17:12
@DanielBrückner Edited; neither is a particularly significant change; given what's all already in place. – Servy Jan 11 '13 at 17:30
Now it will easily deadlock - if a thread has to wait when acquiring the semaphore it will hold the lock on `key` while waiting but this lock is required by any thread wanting to release the semaphore. – Daniel Brückner Jan 11 '13 at 17:49
@DanielBrückner `if a thread has to wait when acquiring the semaphore it will hold the lock on key while waiting` That is incorrect. It will release the lock while it's waiting. You could move the semaphore wait until after the end of the `lock` block though; it wouldn't break the implementation. – Servy Jan 11 '13 at 17:53
No, the lock on `key` will not be released, it is completely unrelated with the semaphore. But moving the wait on the semaphore outside of the lock will indeed (probably) solve the issue. – Daniel Brückner Jan 11 '13 at 18:02
I also just noticed that you never signal the manual reset event; one of the calls to `Reset` is probably intended to by a call to `Set`. – Daniel Brückner Jan 11 '13 at 18:11
Unnaturally I have to say, that there is another issue - something similar as stated for waiting on the semaphore holds for waiting on the manual reset event. The first thread waiting on the manual reset event will keep the lock on `key` and prevent other threads from executing even if they are trying to execute instances of the type matching `lockedType`. – Daniel Brückner Jan 11 '13 at 18:20

Henrik · Answer 3 · 2013-01-11T15:41:26.193

0

Assuming you have a list of all relevant instances, this code will lock on all other instances and thus allow only one instance's code to be executed at any given time:

void MethodA()
{
     foreach (var obj in objects)
         if (obj != this)
             Monitor.Enter(obj);

try
{
    // do stuff
}
finally
{
        foreach( var obj in objects)
        if (obj != this)
                 Monitor.Exit(obj);
}   
}

edited Jan 11 '13 at 15:41

answered Jan 11 '13 at 15:06

Henrik

23,186
6
42
92

This code could very easily deadlock. You need to ensure that your "lock all the things" is logically atomic, which means adding a separate mutex that you lock on when locking on all of the items in the collection. – Servy Jan 11 '13 at 15:07
Also, where is the `objects` collection going to come from? How will he get a hold of all instances of his abstract class? – Servy Jan 11 '13 at 15:10
1

@Servy 1. I don't think it will deadlock, as the locks are always aquired in the same order. 2. he would have to create this list somehow, e.g. insert each object upon construction. – Henrik Jan 11 '13 at 15:20
Could I not create a static field e.g. in the abstract class that holds all instances of all subclasses? Surely I could populate and depopulate that list through the base class constructor each time a subclass get's created or destroyed – cdbeelala89 Jan 11 '13 at 15:20

All threads only in one method at a time?

3 Answers3