Validating asynchronous behavior

Question

I have a large service application I designed; it's a solution made up of about 30 projects.

I was wondering if anyone could point me in the right direction regarding using the Task-based Asynchronous Pattern (TAP) and/or the Event-based Asynchronous Pattern (EAP).

Background

The project in question is installed as a .NET 4.5 Windows service. This service runs a thread that uses a pre-emptive algorithm to execute detached task every 60000ms +/- 20ms (real time).

The service has the ability auto recover, adjust itself based on system load, execute methods based on scheduling criteria in XML.

My Question

Since the service executes (groups of methods/no methods/single method) at specific times from a static context, what is the proper way to validate the asynchronous execution.

I have implemented TPL across the solution, but I am not confident with validating the state of asynchronous running code within this project against a scheduler while implementing the task based asynchronous pattern in a windows service.

In particular handling re-entrancy in Async with parallelism is the main goal.

An Example

The Handler receives a list of methods that need to be run at time (DHHMM). This occurs every minute, at 500ms past the minute. I then asynchronously execute those method(s).

For each method (methodX) I am to track:

• if the method has completed (will return a value indicating if it was a success or failure).
• if the method threw an exception (all exceptions have been handled, but just in case).
• if the method was canceled (due to timeout, or service onShutdown, onPause, etc).
• if the method is still running.

But before I execute any methodX at any time (DHHMM) I need to make sure that methodX is not still currently running from any previous cycle.

I must maintain that one and only one instance of any methodX be running at any time.

• If methodX is called while methodX is still running it is placed on a waiting list for the next interval.
• If methodX is called while methodX has failed or thrown an exception previously it is placed on a blocking list for all intervals(until removed from blocklist).

I came up with a solution using a static dictionary to manage the status of tasks, but this is becoming apparently complicated, as I try to ensure mutual exclusion of the data structure. As I code the taskexecuter, I am attempting to batch execute collections of task based functions. I have extensively read about implementing the EAP, or current TAP. I will update the question as I code.

namespace ServiceTestFloor
{
    public class TestFloor2
    {
        public static List<Func<bool, bool>> lstActions = TestFloor2A.getMethodsEWS();
        public static void ExecuteTask(string processname)
        {
            // if processname is in the list of functions then we fire the task
            /* adding more fuctionality requires external processes, or a re-complie with new code */
            foreach (Func<bool, bool> act in lstActions)
            {
                string strclassname = act.Method.Name; // delay for the main action for this contract
                if (strclassname == processname)
                {
                    bool isValid = act(true);
                    return;
                }
            }
        }

        public static void init()
        {
            Console.WriteLine("Running TestFloor2A.cs");
            initalizeMonitor();
            Console.WriteLine("Monitor initalized");

            // set up a program to pass lists of functions to test the process handler.

            // list one, sleep, list 2, sleep, list 3, sleep, list 4, sleep
            List<string> testgroup = new List<string>();
        }

        public static void initalizeMonitor()
        {
            /* Clears the monitor */
            dTMx = new Dictionary<string, int[]>();
            isActive = false;
            blocklist = new List<string>();
            waitlist = new List<string>();
        }

        private static bool isActive;
        private static List<string> recientCompleted = new List<string>();
        private static int intervalFailBlocking = 60; /* if a process failed then reset the failed list after an hour*/

        private static List<string> waitlist = new List<string>();
        private static List<string> blocklist = new List<string>(); /* blocking for the master process */
        private static Dictionary<string, int[]> dTMx = new Dictionary<string, int[]>();

        /*
         * taskstatus
         * Key(string)          =   process name
         * Value(int[])
         * inx[0]               =   process status      0 = Complete, 1 = Incomplete, 2 = Failed, 3 = Canceled
         * inx[1]               =   Rcycs repetitions cycle counter
         * inx[2]               =   Wcycs waiting cycle counter
         * inx[3]               =   Fcycs failed cycle counter
         * inx[4]               =   Ccycs canceled cycle counterss
         */

        public static void ReportProcessCompleted(string processname)
        {
            if (dTMx.ContainsKey(processname))
            {
                int[] inxx = dTMx[processname];
                inxx[0] = 0;
                dTMx[processname] = inxx;
                //recientCompleted.Add(processname);
                Console.WriteLine("Process Complete:" + processname + "   Rcycle time: " + inxx[1]);
            }
            else
            {
                Console.WriteLine("ReportProcessCompleted Failed:" + processname + "   Not Found");

            }
        }

        public static void ReportProcessFailed(string processname)
        {
            if (dTMx.ContainsKey(processname))
            {
                int[] inxx = dTMx[processname];
                inxx[0] = 2;
                dTMx[processname] = inxx;
                Console.WriteLine("Process Failed:" + processname + "   Rcycle time: " + inxx[1]);
            }
            else
            {
                Console.WriteLine("ReportProcessFailed Failed:" + processname + "   Not Found");
            }
        }

        public static void ReportProcessCancel(string processname)
        {
            if (dTMx.ContainsKey(processname))
            {
                int[] inxx = dTMx[processname];
                inxx[0] = 3;
                dTMx[processname] = inxx;
                Console.WriteLine("Process Canceled:" + processname + "   Rcycle time: " + inxx[1]);
            }
            else
            {
                Console.WriteLine("ReportProcessCancel Failed:" + processname + "   Not Found");

            }
        }

        private static void processPre() /* Preemptive actions */
        {
            foreach (KeyValuePair<string, int[]> entry in dTMx) /* iterate the dTMx dictionary */
            {
                int[] inx = entry.Value;
                string processname = entry.Key;
                if (inx[0] == 0)
                {
                    Console.WriteLine("Process Completed:" + processname + "   Removing from Master");
                    dTMx.Remove(processname); /* process completed sucessfully so remove from monitored processes */
                }

            }
            if (dTMx.Count == 0)
            {
                isActive = false;
            }
            else
            {
                isActive = true;
            }
        }

        public static void processMain(List<string> pls) /* calling method, main action handler */
        {
            processPre();
            List<string> tcci = pls;
            List<string> tcco = new List<string>(); /* output combinations */

            Console.WriteLine("Items in Input List");
            foreach (string str in tcci)
            {
                Console.WriteLine(str);
            }
            Console.WriteLine();

            if (waitlist != null)
            {
                Console.WriteLine("Merging Wait List with Input");
                Console.WriteLine("Items in Wait List");
                foreach (string str in waitlist)
                {
                    Console.WriteLine(str);
                }
                Console.WriteLine();
                tcco = tcci.Union(waitlist).ToList();

                Console.WriteLine("Items in Merged Input List");
                foreach (string str in tcco)
                {
                    Console.WriteLine(str);
                }
                Console.WriteLine();
                waitlist = new List<string>();
                Console.WriteLine("Wait List Cleared");
            }
            else
            {
                Console.WriteLine("Wait List Empty");

            }

            if (blocklist != null)
            {
                Console.WriteLine("Removing Blocked Items");
                Console.WriteLine("Items in Block List");
                foreach (string str in blocklist)
                {
                    Console.WriteLine(str);
                }
                Console.WriteLine();

                tcco = tcco.Except(blocklist).ToList();

                Console.WriteLine("Excepted Output List");
                foreach (string str in tcco)
                {
                    Console.WriteLine(str);
                }
                Console.WriteLine();
            }
            else
            {
                Console.WriteLine("Block List Empty");

            }

            Console.WriteLine("Finalized List");
            foreach (string str in tcco)
            {
                Console.WriteLine(str);
            }
            Console.WriteLine();

            if (pls != null || pls.All(x => string.IsNullOrWhiteSpace(x))) /*check if list is empty or invalid */
            {
                /* input list has processes */
                foreach (string proc in tcco)
                {
                    int[] inxx = { 1, 0, 0, 0, 0 };
                    if (dTMx.ContainsKey(proc))
                    {
                        /* update the process if complete, canceled, Failed
                         * inx[0]               =   process status      0 = Complete, 1 = Incomplete, 2 = Failed, 3 = Canceled
                         * inx[1]               =   Rcycs repetitions cycle counter
                         * inx[2]               =   Wcycs waiting cycle counter
                         * inx[3]               =   Fcycs failed cycle counter
                         * inx[4]               =   Ccycs canceled cycle counter
                         */

                        inxx = dTMx[proc];
                        if (inxx[0] == 0) //0 = Complete, 1 = Incomplete, 2 = Failed, 3 = Canceled
                        {
                            dTMx[proc] = inxx;

                        }
                        else if (inxx[0] == 1) /* incomplete code, inc the Wcycs++ */
                        {
                            int Wcycs = inxx[2];
                            Wcycs++;
                            inxx[2] = Wcycs;
                            dTMx[proc] = inxx;
                            addtoWaitList(proc);
                        }
                        else if (inxx[0] == 2) /* failed code, inc the Fcycs++ */
                        {
                            int Fcycs = inxx[3];
                            Fcycs++;
                            inxx[3] = Fcycs;
                            dTMx[proc] = inxx;

                        }
                        else if (inxx[0] == 3)
                        {
                            int Ccycs = inxx[4];
                            Ccycs++;
                            inxx[4] = Ccycs;
                            dTMx[proc] = inxx;
                        }


                    }
                    else
                    {
                        /* add the process as incomplete */
                        //dTMx.Add(proc, inxx);
                        addProcess(proc);
                    }
                }
            }
            else
            {
                /* input list is empty */
            }
            processPost();
        }


        private static void processPost() /* Postemptive actions */
        {

            incrementRCycs();
            /* after setting up new tasks and registering the tasks */
            /* now check the tasks that have registered cancel if any one exceed 5 failures in a row, 5 cancels then alert the necessary */
            List<string> ffailedlist = new List<string>();
            List<string> ccancellist = new List<string>();
            List<string> wwaitedlist = new List<string>();
            foreach (KeyValuePair<string, int[]> entry in dTMx) /* iterate the dTMx dictionary */
            {
                int[] inx = entry.Value;
                string processname = entry.Key;
                int Fcycs = inx[3];
                int Wcycs = inx[2];
                int Ccycs = inx[4];
                if (Fcycs > 5)
                {
                    ffailedlist.Add(processname);

                }
                if (Wcycs > 5)
                {
                    wwaitedlist.Add(processname);

                }
                if (Ccycs > 5)
                {
                    ccancellist.Add(processname);

                }
            }

            // if an item is waited for 5 times, it may be a long running process, after 5 times
            Console.WriteLine("----------------------------");
            Console.WriteLine("Items in ffailedlist");
            foreach (string str in ffailedlist)
            {
                Console.WriteLine(str);
            }
            Console.WriteLine();
            Console.WriteLine("Items in wwaitedlist");
            foreach (string str in wwaitedlist)
            {
                Console.WriteLine(str);
            }
            Console.WriteLine();
            Console.WriteLine("Items in ccancellist");
            foreach (string str in ccancellist)
            {
                Console.WriteLine(str);
            }
            Console.WriteLine();
        }


        private static void incrementRCycs()
        {
            foreach (KeyValuePair<string, int[]> entry in dTMx) /* iterate the dTMx dictionary */
            {
                int[] inx = entry.Value;
                string processname = entry.Key;
                int Rcycs = inx[1];
                Rcycs++;
                inx[1] = Rcycs;
                dTMx[processname] = inx;

            }
        }

        private static void addtoWaitList(string processname)
        {

            if (waitlist.Contains(processname))
            {
                /* ignore this access attempt */
                Console.WriteLine("Process.Add WaitList: " + processname + " Was Skipped, becuase it was already in the list");
            }
            else
            {
                waitlist.Add(processname);
            }

        }

        private static void addProcess(string processname) /* returns stats of processname 0 = complete, 1 = incomplete, 2 = Failed, 3 = canceled, 7 = empty */
        {
            if (dTMx.ContainsKey(processname))
            {
                /* ignore this access attempt */
                Console.WriteLine("Process.Add Master: " + processname + " Was Skipped, becuase it was already in the list");
            }
            else
            {
                ExecuteTask(processname);
                int[] inxx = { 1, 0, 0, 0, 0 };
                dTMx.Add(processname, inxx);

            }

        }


        private static int getStatus(string processname) /* returns stats of processname 0 = complete, 1 = incomplete, 2 = Failed, 3 = canceled, 7 = empty */
        {
            int x = 7;
            if (dTMx.ContainsKey(processname))
            {
                int[] inxx = dTMx[processname];
                x = inxx[0];
            }
            return x;
        }            
    }
}

Answer 1

Well, I'm going to concentrate on the basics for now:

Naming Conventions

1. In C# methods are generally PascalCase same goes for public fields and properties.

2. Prefixing names with their type like lstActions or strclassname (also known as hungarian notation) generally provides no value and just creates clutter which decreases readability.

3. Please use sensible names for variables, fields etc. dMTx, inxx, tcci, tcci, Rcys, ... etc. That just nuts. A name should convey the purpose of the variable/field - what is it being used for and what kind of information does it hold - in a concise but readable manner. Maintainability is about someone coming back to it 6 or 12 months later and still being able to understand what that stuff all means.

General Code

This:

  /* update the process if complete, canceled, Failed
     * inx[0]               =   process status      0 = Complete, 1 = Incomplete, 2 = Failed, 3 = Canceled
     * inx[1]               =   Rcycs repetitions cycle counter
     * inx[2]               =   Wcycs waiting cycle counter
     * inx[3]               =   Fcycs failed cycle counter
     * inx[4]               =   Ccycs canceled cycle counter
     */

is, erm, let's say suboptimal. You have an object oriented language - use it. I wouldn't even accept this in plain old C code. Something like this:

enum ProcessState
{
    Complete = 0,
    Incomplete = 1,
    Failed = 2,
    Canceled = 3
}

class ProcessInfo
{
    public ProcessState State { get; set; }
    public int RepeatCycles { get; set; }
    public int WaitCycles { get; set; }
    public int FailedCycles { get; set; }
    public int CanceledCycles { get; set; }
}

A lot easier to read, understand and maintain. No magic numbers. No comments necessary.

---

You seem to do a lot of unnecessary assignments like this:

int[] inxx = dTMx[processname];
 inxx[0] = 2;
 dTMx[processname] = inxx;

which is fully equivalent to:

dTMx[processname][0] = 2;

---

Quite a bit of code is duplicated. For example ReportProcessCompleted, ReportProcessFailed and ReportProcessCanceled are pretty much the same bar the different state it sets and the state name it logs.

Code duplication is bad since if you want to change the way how the code acts to state changes (log differently or do something else like raising an event) you have to change it in multiple places instead of just one.

Design

As other have already mentioned - everything is static which in generally is a bad idea. There are many implicit dependencies which are not clear until you read the code. The public interface is ill-defined and it's unclear what methods should be called when and if there should be any particular order to things.

You mentioned that this is called from 22 locations in the solution - you and whoever else is working on the project will be in for a lot of pain in the long run. Static classes with static state used all over the place will increase coupling and create an utterly brittle system which falls apart at the slightest change. I've seen this happen in more than one code base and it's a massive amount of work to backtrack from this. You should get this right from the start.

Also you said something about mutual exclusion - this makes me wonder if you are talking about multi threaded access. Currently your code will explode if used in a multi threaded environment. There is no locking of shared state anywhere.

Answer 2

Although you got some good pointers from @ChrisWue's answer I will throw my 5ct into this too.

Dictionary usage

If you not only need to check if a key is contained in a dictionary but also need the value, you shouldn't us ContainsKey() but TryGetValue().

Using your approach is first checking if the key is found in the dictionary. If that is true the access to the value by using the dictionary Item getter involves another check for the key. By using TryGetValue() this check is only done once.

For instance this

public static void ReportProcessCancel(string processname)
{
    if (dTMx.ContainsKey(processname))
    {
        int[] inxx = dTMx[processname];
        inxx[0] = 3;
        dTMx[processname] = inxx;
        Console.WriteLine("Process Canceled:" + processname + "   Rcycle time: " + inxx[1]);
    }
    else
    {
        Console.WriteLine("ReportProcessCancel Failed:" + processname + "   Not Found");

    }
}  

should be changed to this

public static void ReportProcessCancel(string processname)
{
    int[] value;
    if (dTMx.TryGetValue(processname, out value))
    {
        value[0] = 3;
        Console.WriteLine("Process Canceled:" + processname + "   Rcycle time: " + value[1]);
    }
    else
    {
        Console.WriteLine("ReportProcessCancel Failed:" + processname + "   Not Found");

    }
}  

As you see I don't assign the int[] value back to the dictionary, because it is an object (array) so it only holds a reference to that array which is contained in the dictionary.

You should also use the overloaded WriteLine() method of the Console which uses formatted values like so

public static void ReportProcessCancel(string processname)
{
    int[] value;
    if (dTMx.TryGetValue(processname, out value))
    {
        value[0] = 3;
        Console.WriteLine("Process Canceled:{0}   Rcycle time: {1}" , processname, value[1]);
    }
    else
    {
        Console.WriteLine("ReportProcessCancel Failed:{0}   Not Found", processname);

    }
}  

---

private static void processPre() /* Preemptive actions */
{
    foreach (KeyValuePair<string, int[]> entry in dTMx) /* iterate the dTMx dictionary */
    {
        int[] inx = entry.Value;
        string processname = entry.Key;
        if (inx[0] == 0)
        {
            Console.WriteLine("Process Completed:" + processname + "   Removing from Master");
            dTMx.Remove(processname); /* process completed sucessfully so remove from monitored processes */
        }

    }
    if (dTMx.Count == 0)
    {
        isActive = false;
    }
    else
    {
        isActive = true;
    }
}  

this can be enhanced by just setting isActive = dTMx.Count !=0; and by using some "linq magic" and the usage of the var type like so

private static void processPre() /* Preemptive actions */
{
    foreach (var entry in dTMx.Where(kvp => kvp.Value[0] == 0).ToList())
    {
        Console.WriteLine("Process Completed:   Removing from Master", entry.Key);
        dTMx.Remove(entry.Key); 
    }

    isActive = dTMx.Count != 0;
}  

---

private static void incrementRCycs()
{
    foreach (KeyValuePair<string, int[]> entry in dTMx) /* iterate the dTMx dictionary */
    {
        int[] inx = entry.Value;
        string processname = entry.Key;
        int Rcycs = inx[1];
        Rcycs++;
        inx[1] = Rcycs;
        dTMx[processname] = inx;

    }
}  

this is a good example how you shouldn't use a Dictionary<TKey, TValue either. It can be shortened to

private static void incrementRCycs()
{
    foreach (KeyValuePair<string, int[]> entry in dTMx)
    {
        entry.Value[1] += 1;
    }
}  

---

The processMain() method is way to long and has to many responsibilities. You should at least extract the output of the collections to a separate method and maybe refactor it to some more methods.