Validating asynchronous behavior

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 interva

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

Naming Conventions

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

-
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.

-
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.

StackExchange Code Review Q#59838, answer score: 3