So what do you think is the best way to prevent multiple threads of a C# Windows service running simultaneously (the service is using a timer with the OnElapsed event) ?
Using lock() or mutex ?
I can't seem to grasp the concept of the mutex, but using lock() seems to work fine for my case.
Should I spend the time learning how to use the mutex anyways?
Make your timer a one-shot, and re-initialize it in the elapsed event handler. For example, if you're using System.Timers.Timer, you'd initialize it like this:
myTimer.Elapsed = timer1Elapsed;
myTimer.Interval = 1000; // every second
myTimer.AutoReset = false; // makes it fire only once
myTimer.Enabled = true;
And your elapsed event handler:
void timerElapsed(object source, ElapsedEventArgs e)
{
// do whatever needs to be done
myTimer.Start(); // re-enables the timer
}
The drawback to this is that the timer doesn't fire on one second intervals. Rather, it fires one second after the last tick's processing finishes.
Don't use a timer to spawn threads. Only ever start one thread. When the thread has finished a work cycle, calculate how long remains before the next cycle should start. If this interval is 0 or negative, loop back immediately and start a new cycle, if positive, sleep for that interval before looping back.
This is usually done by taking the int result of an unsigned int subtraction between the finish ticks and start ticks, so giving the elapsed ticks taken by the work. Subtracting this from the desired interval gives the new time remaining.
No extra timer thread needed, no possibility of two threads running simultaneously, simplified overall design, no continual create/start/terminate/destroy, no mallocs, no new(), no stack allocate/deallocate, no GC.
Other designs using timers, mutexes, semaphores, locks etc. are just over complex. Why bother trying to stop the extra threads with synchro if it's just plain easier and simpler to not make any extra threads?
Sometimes, using a timer instead of a sleep() loop is just a really bad idea. This sounds like one of those times.
public void doWorkEvery(int interval)
{
while (true)
{
uint startTicks;
int workTicks, remainingTicks;
startTicks = (uint)Environment.TickCount;
DoSomeWork();
workTicks=(int)((uint)Environment.TickCount-startTicks);
remainingTicks = interval - workTicks;
if (remainingTicks>0) Thread.Sleep(remainingTicks);
}
}
Instead of lock you can use Monitor.TryEnter() to return if a callback is already being executed by another timer thread:
class Program
{
static void Main(string[] args)
{
Timer t = new Timer(TimerCallback, null,0,2000);
Console.ReadKey();
}
static object timerLock = new object();
static void TimerCallback(object state)
{
int tid = Thread.CurrentThread.ManagedThreadId;
bool lockTaken = false;
try
{
lockTaken = Monitor.TryEnter(timerLock);
if (lockTaken)
{
Console.WriteLine("[{0:D02}]: Task started", tid);
Thread.Sleep(3000); // Do the work
Console.WriteLine("[{0:D02}]: Task finished", tid);
}
else
{
Console.WriteLine("[{0:D02}]: Task is already running", tid);
}
}
finally
{
if (lockTaken) Monitor.Exit(timerLock);
}
}
}
I think I know what you're trying to do. You've got a timer that executes a callback periodically (definition of a timer) and that callback does a bit of work. that bit of work could actually take more time than the timer period (e.g. the timer period is 500 ms and a given invocation of your callback could take longer that 500 ms). This means that your callback needs to be re-entrant.
If you can't be re-entrant (and there's various reasons why this might be); what I've done in the past is to turn off the timer at the start of the callback then turn it back on at the end. For example:
private void timer_Elapsed(object source, ElapsedEventArgs e)
{
timer.Enabled = false;
//... do work
timer.Enabled = true;
}
If you want to actually want one "thread" to execute immediately after another, I wouldn't suggest using a timer; I would suggest using Task objects. For example
Task.Factory.StartNew(()=>{
// do some work
})
.ContinueWith(t=>{
// do some more work without running at the same time as the previous
});
If all you want is to prevent two threads in the same process/app domain from executing concurrently, the lock statement will probably do for you.
But note that lock leaves the other threads, well, locked while they wait for access to the critical section. They are not aborted or redirected or anything; they are sitting there, waiting for the original thread to finish execution of the lock block so they may run.
A mutex would give you greater control; including the ability to have second and subsequent threads just stop altogether, rather than locking, and locking threads across processes.
I think that some of these approaches are fantastic, but a bit complicated.
I've created a wrapper class that prevents a timer from overlapping and allows you to choose whether "ELAPSED should be called once every INTERVAL" or "An INTERVAL delay should occur between calls".
If you improve on this code, please post the updates here!
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace AllCommander.Diagnostics {
public class SafeTimer : IDisposable {
public enum IntervalStartTime {
ElapsedStart,
ElapsedFinish
}
private System.Timers.Timer InternalTimer;
public bool AutoReset { get; set; }
public bool Enabled {
get {
return InternalTimer.Enabled;
}
set {
if (value) {
Start();
} else {
Stop();
}
}
}
private double __Interval;
public double Interval {
get {
return __Interval;
}
set {
__Interval = value;
InternalTimer.Interval = value;
}
}
/// <summary>
/// Does the internal start ticking at the END of Elapsed or at the Beginning?
/// </summary>
public IntervalStartTime IntervalStartsAt { get; set; }
public event System.Timers.ElapsedEventHandler Elapsed;
public SafeTimer() {
InternalTimer = new System.Timers.Timer();
InternalTimer.AutoReset = false;
InternalTimer.Elapsed += InternalTimer_Elapsed;
AutoReset = true;
Enabled = false;
Interval = 1000;
IntervalStartsAt = IntervalStartTime.ElapsedStart;
}
void InternalTimer_Elapsed(object sender, System.Timers.ElapsedEventArgs e) {
if (Elapsed != null) {
Elapsed(sender, e);
}
var ElapsedTime = DateTime.Now - e.SignalTime;
if (AutoReset == true) {
//Our default interval will be INTERVAL ms after Elapsed finished.
var NewInterval = Interval;
if (IntervalStartsAt == IntervalStartTime.ElapsedStart) {
//If ElapsedStart is set to TRUE, do some fancy math to determine the new interval.
//If Interval - Elapsed is Positive, then that amount of time is remaining for the interval
//If it is zero or negative, we're behind schedule and should start immediately.
NewInterval = Math.Max(1, Interval - ElapsedTime.TotalMilliseconds);
}
InternalTimer.Interval = NewInterval;
InternalTimer.Start();
}
}
public void Start() {
Start(true);
}
public void Start(bool Immediately) {
var TimerInterval = (Immediately ? 1 : Interval);
InternalTimer.Interval = TimerInterval;
InternalTimer.Start();
}
public void Stop() {
InternalTimer.Stop();
}
#region Dispose Code
//Copied from https://lostechies.com/chrispatterson/2012/11/29/idisposable-done-right/
bool _disposed;
public void Dispose() {
Dispose(true);
GC.SuppressFinalize(this);
}
~SafeTimer() {
Dispose(false);
}
protected virtual void Dispose(bool disposing) {
if (!_disposed) {
if (disposing) {
InternalTimer.Dispose();
}
// release any unmanaged objects
// set the object references to null
_disposed = true;
}
}
#endregion
}
}
