I have implemented some methods to make the creation of multithreaded programs easier for me in the future

The requirement was:

• Be able to add as many Tasks as I need to the pool
• Be able to restrict the amount of concurrently running threads
• Be able to set the thread creation speed

This is the code i came up with, note I do not know the conventions of Thread safety and design patterns usually used in multithreaded programming in c# or any other language.

This is a first attempt at a topic im interested in, The point of the review is to see if there are improvements I can make to my code and learn some new methods to make it tidier and easier to understand

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace MTSimplified
{
public class NotStartedException : Exception { public NotStartedException(){ } }

{

private bool keepCreating,started;
private CancellationTokenSource cts;

/* entire pool */
public event EventHandler onDone;
public event EventHandler onStopped;

{
this.creationInterval = creationInterval;
this.cts = new CancellationTokenSource();
this.keepCreating = true;
this.started = false;
}

{
if (started)

{
functionWrapper(function,cts.Token);
}));
}

public void startAll()
{
if (started)

this.started = true;
{
{
}
}

if (!keepCreating)
return;

while (threadList.Count(t => t.IsAlive) != 0) ;

onDone?.Invoke(this, EventArgs.Empty);

});
}

public void stopAll()
{
if (!started)
throw new NotStartedException();

this.cts.Cancel();
this.keepCreating = false;

while (threadList.Count(t => t.IsAlive) != 0) ;

onStopped?.Invoke(this, EventArgs.Empty);
}

/* private methods */
private void functionWrapper(Action function, CancellationToken ct)
{
function.Invoke();
}

{

}

}
}


To test the class

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace MTSimplified
{
class Program
{
static int i = 0;

static void Main(string[] args)
{

threadO.onStopped += (s, e) => canceled();

for (int i = 0; i<200;i++)

while (true) ;
}

public static void print()
{
i++;
Console.WriteLine(i);

}
public static void created()
{
Console.WriteLine("created");
}
public static void finished()
{
Console.WriteLine("finished");
}
public static void done()
{
Console.WriteLine("Everything is done");
}
public static void canceled()
{
Console.WriteLine("Everything is canceled");
}

}
}


I don't have time to write a better implementation of a thread pool, but I'll point out what I see is wrong.

I'm quite sure your implementation does not satisfy the requirements' intention. Where does it say that none of the tasks should be started when added, and then they should only be started all at once (but not really, because of the delay between thread creation)? And once the "thread pool" has been started, you can't add any further tasks? As it stands, I cannot see any real use for it. Not that I can see a real use for a better version either, since you can pretty much do all that with the standard thread pool.

Other issues:

• Each functionWrapper method runs on a separate thread, and increments finishedThreads when complete. This is not thread safe, and you're bound to lose increments. Given that you use this variable in your while loop in startAll(), this is a problem. Perhaps use Interlocked.Increment.
• Speaking of which, finishedThreads, currentThreadIndex, and keepCreating should be marked volatile so that the reads of their values aren't hoisted out of the while loop.
• I don't believe your implementation meets the requirement "Be able to cancel [...] currently running threads". I can't test it myself but I'm pretty sure that simply calling Cancel() on a CancellationTokenSource when the function you started is already running will do a whole load of nothing, unless you pass the token in to the method, and then continually test it from within the method (in this case, from within the method that gets invoked when function.Invoke() is called in functionWrapper). Of course, if the method being invoked doesn't care about CancellationTokens, then you're plum out of luck - it's not "cancellable" (apart from aborting the thread). Someone may correct me but I believe that's how it is.
• Use PascalCase for method names and public events.
• Put a space after a comma (e.g. in private int maxThreads,creationInterval,finishedThreads,currentThreadIndex;)
• Be consistent: startAll() uses a member field like so: if (started) ... and two lines later uses "this": this.started = true;. Do one or the other.
while (threadList.Count(t => t.IsAlive) != 0) ;


this line will burn up CPU, may be a very long time if a thread doesn't exit.