C# 2.0 Windows Service - Memory Leak? [modified]
-
I have a windows service in C# .net 2.0. At the moment all it is doing is being instantiated, setting up a timer and then every interval (eg 1 minute) writing to a log file. The memory usage on TaskManager goes up and up ... I can't see any objects that I've left lying around - I've taken out pretty much everything it is meant to do ... perhaps I'm being paranoid and the garbage collector will come and clean up when it is ready? How long should I leave it? It's been a while since the last Windows Service I wrote ... any ideas? Thanks in advance :) Here are some excerpts from the code... The LogFile is a static class with a single static method "Write"
public static class LogFile { public static void Write(string message) { string logFileName = (String)(new AppSettingsReader()).GetValue("log-file-path", typeof(String)) + "FileLoaderServiceLog.txt"; File.AppendAllText(logFileName, DateTime.Now.ToString("dd-MMM-yyyy HH:mm:ss") + " FileLoaderService:" + " " + message + Environment.NewLine); }
The Service class itself
public partial class FileLoaderService : ServiceBase { private Timer serviceTimer; public FileLoaderService() { InitializeComponent(); serviceTimer = new Timer(); double intervalInMinutes = (double)(new AppSettingsReader()).GetValue("timer-interval-in-minutes", typeof(double)); serviceTimer.Interval = intervalInMinutes \* 60 \* 1000; //intervalInMinutes \* 60 seconds \* 1000ms serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer\_Elapsed); } protected override void OnStart(string\[\] args) { LogFile.Write("Service Started - starting timer"); serviceTimer.Start(); } protected void serviceTimer\_Elapsed(object sender, ElapsedEventArgs e) { //stop timer whilst we process serviceTimer.Stop(); //do processing //have removed this to try to find the "leak" LogFile.Write("Checking Queue..."); //processing done - start it up again serviceTimer.Start(); } protected override void OnStop() { serviceTimer.Stop(); serviceTimer = null; LogFile.Write("Service Stopped"); } }
And Program Main ...
static class Program {
-
I have a windows service in C# .net 2.0. At the moment all it is doing is being instantiated, setting up a timer and then every interval (eg 1 minute) writing to a log file. The memory usage on TaskManager goes up and up ... I can't see any objects that I've left lying around - I've taken out pretty much everything it is meant to do ... perhaps I'm being paranoid and the garbage collector will come and clean up when it is ready? How long should I leave it? It's been a while since the last Windows Service I wrote ... any ideas? Thanks in advance :) Here are some excerpts from the code... The LogFile is a static class with a single static method "Write"
public static class LogFile { public static void Write(string message) { string logFileName = (String)(new AppSettingsReader()).GetValue("log-file-path", typeof(String)) + "FileLoaderServiceLog.txt"; File.AppendAllText(logFileName, DateTime.Now.ToString("dd-MMM-yyyy HH:mm:ss") + " FileLoaderService:" + " " + message + Environment.NewLine); }
The Service class itself
public partial class FileLoaderService : ServiceBase { private Timer serviceTimer; public FileLoaderService() { InitializeComponent(); serviceTimer = new Timer(); double intervalInMinutes = (double)(new AppSettingsReader()).GetValue("timer-interval-in-minutes", typeof(double)); serviceTimer.Interval = intervalInMinutes \* 60 \* 1000; //intervalInMinutes \* 60 seconds \* 1000ms serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer\_Elapsed); } protected override void OnStart(string\[\] args) { LogFile.Write("Service Started - starting timer"); serviceTimer.Start(); } protected void serviceTimer\_Elapsed(object sender, ElapsedEventArgs e) { //stop timer whilst we process serviceTimer.Stop(); //do processing //have removed this to try to find the "leak" LogFile.Write("Checking Queue..."); //processing done - start it up again serviceTimer.Start(); } protected override void OnStop() { serviceTimer.Stop(); serviceTimer = null; LogFile.Write("Service Stopped"); } }
And Program Main ...
static class Program {
In your OnStop method, you should Dispose of the timer rather than set it to null. As far as the rest goes, the garbage collector will run when it needs to. You shouldn't need to get the log filename every time, get it once and store it in a static field.
"WPF has many lovers. It's a veritable porn star!" - Josh Smith
As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.
-
I have a windows service in C# .net 2.0. At the moment all it is doing is being instantiated, setting up a timer and then every interval (eg 1 minute) writing to a log file. The memory usage on TaskManager goes up and up ... I can't see any objects that I've left lying around - I've taken out pretty much everything it is meant to do ... perhaps I'm being paranoid and the garbage collector will come and clean up when it is ready? How long should I leave it? It's been a while since the last Windows Service I wrote ... any ideas? Thanks in advance :) Here are some excerpts from the code... The LogFile is a static class with a single static method "Write"
public static class LogFile { public static void Write(string message) { string logFileName = (String)(new AppSettingsReader()).GetValue("log-file-path", typeof(String)) + "FileLoaderServiceLog.txt"; File.AppendAllText(logFileName, DateTime.Now.ToString("dd-MMM-yyyy HH:mm:ss") + " FileLoaderService:" + " " + message + Environment.NewLine); }
The Service class itself
public partial class FileLoaderService : ServiceBase { private Timer serviceTimer; public FileLoaderService() { InitializeComponent(); serviceTimer = new Timer(); double intervalInMinutes = (double)(new AppSettingsReader()).GetValue("timer-interval-in-minutes", typeof(double)); serviceTimer.Interval = intervalInMinutes \* 60 \* 1000; //intervalInMinutes \* 60 seconds \* 1000ms serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer\_Elapsed); } protected override void OnStart(string\[\] args) { LogFile.Write("Service Started - starting timer"); serviceTimer.Start(); } protected void serviceTimer\_Elapsed(object sender, ElapsedEventArgs e) { //stop timer whilst we process serviceTimer.Stop(); //do processing //have removed this to try to find the "leak" LogFile.Write("Checking Queue..."); //processing done - start it up again serviceTimer.Start(); } protected override void OnStop() { serviceTimer.Stop(); serviceTimer = null; LogFile.Write("Service Stopped"); } }
And Program Main ...
static class Program {
If you want to make it easier on yourself for debugging, move the code into a separate assembly, and debug it from a console app. That's the way I develop windows services. Once I've got the core code working correctly, I then write the service assembly itself that references the core code assembly, and anything that goes wrong at that point is the fault of the service code itself. As to your problem, I don't use the Timer objects. I prefer to set up a thread that does the timer work, and another thread that performs the actual work. This way, you can see if the worker thread is busy and if so, you can just ignore the request to write the log file (or even queue it up for processing later). If you want to save some processing time, retrieve your settings just one time, which brings to the forefront your
AppSettingsReader
object. Could that be one of the the sources of your memory leak? Lastly, if an object you're creating has a Dispose method (like the Timer objects do), call it before setting the object to null..45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001 -
I have a windows service in C# .net 2.0. At the moment all it is doing is being instantiated, setting up a timer and then every interval (eg 1 minute) writing to a log file. The memory usage on TaskManager goes up and up ... I can't see any objects that I've left lying around - I've taken out pretty much everything it is meant to do ... perhaps I'm being paranoid and the garbage collector will come and clean up when it is ready? How long should I leave it? It's been a while since the last Windows Service I wrote ... any ideas? Thanks in advance :) Here are some excerpts from the code... The LogFile is a static class with a single static method "Write"
public static class LogFile { public static void Write(string message) { string logFileName = (String)(new AppSettingsReader()).GetValue("log-file-path", typeof(String)) + "FileLoaderServiceLog.txt"; File.AppendAllText(logFileName, DateTime.Now.ToString("dd-MMM-yyyy HH:mm:ss") + " FileLoaderService:" + " " + message + Environment.NewLine); }
The Service class itself
public partial class FileLoaderService : ServiceBase { private Timer serviceTimer; public FileLoaderService() { InitializeComponent(); serviceTimer = new Timer(); double intervalInMinutes = (double)(new AppSettingsReader()).GetValue("timer-interval-in-minutes", typeof(double)); serviceTimer.Interval = intervalInMinutes \* 60 \* 1000; //intervalInMinutes \* 60 seconds \* 1000ms serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer\_Elapsed); } protected override void OnStart(string\[\] args) { LogFile.Write("Service Started - starting timer"); serviceTimer.Start(); } protected void serviceTimer\_Elapsed(object sender, ElapsedEventArgs e) { //stop timer whilst we process serviceTimer.Stop(); //do processing //have removed this to try to find the "leak" LogFile.Write("Checking Queue..."); //processing done - start it up again serviceTimer.Start(); } protected override void OnStop() { serviceTimer.Stop(); serviceTimer = null; LogFile.Write("Service Stopped"); } }
And Program Main ...
static class Program {
Most likely, it's that you keep instantiating the AppSettingsReader in the LogFile.Write method; you shouldn't need to to that. Everything else looks OK to me (it's the same way I write Windows Services), but I strongly suggest a try/catch/finally in the serviceTimer_Elapsed method. Edit: Oh, wait, don't throw away the Timer in OnStop, just leave it stopped. If you want, you can write a Dispose for the FileLoaderService that will dispose of the Timer. Otherwise, you should instantiate the Timer in OnStart. The reason for this is if you ever do have multiple Services hosted in one process. With your current implementation, you can't stop and restart just one.
-
Most likely, it's that you keep instantiating the AppSettingsReader in the LogFile.Write method; you shouldn't need to to that. Everything else looks OK to me (it's the same way I write Windows Services), but I strongly suggest a try/catch/finally in the serviceTimer_Elapsed method. Edit: Oh, wait, don't throw away the Timer in OnStop, just leave it stopped. If you want, you can write a Dispose for the FileLoaderService that will dispose of the Timer. Otherwise, you should instantiate the Timer in OnStart. The reason for this is if you ever do have multiple Services hosted in one process. With your current implementation, you can't stop and restart just one.
Thanks for your reply :) I see what you mean about the AppSetttingsReader ... I had originally called it once in the constructor but changed it after some advice from a friend. We both figured that because it was a null reference it should be cleaned up ... nevertheless it is better form to only get it once - I agree. And .. I get what you mean about the OnStop ... Thanks :)
-
If you want to make it easier on yourself for debugging, move the code into a separate assembly, and debug it from a console app. That's the way I develop windows services. Once I've got the core code working correctly, I then write the service assembly itself that references the core code assembly, and anything that goes wrong at that point is the fault of the service code itself. As to your problem, I don't use the Timer objects. I prefer to set up a thread that does the timer work, and another thread that performs the actual work. This way, you can see if the worker thread is busy and if so, you can just ignore the request to write the log file (or even queue it up for processing later). If you want to save some processing time, retrieve your settings just one time, which brings to the forefront your
AppSettingsReader
object. Could that be one of the the sources of your memory leak? Lastly, if an object you're creating has a Dispose method (like the Timer objects do), call it before setting the object to null..45 ACP - because shooting twice is just silly
-----
"Why don't you tie a kerosene-soaked rag around your ankles so the ants won't climb up and eat your candy ass..." - Dale Earnhardt, 1997
-----
"The staggering layers of obscenity in your statement make it a work of art on so many levels." - J. Jystad, 2001I thought about using Threads but decided it was complexity that my future replacement just didn't need. When it comes down to it ... should the service return to a low level of memory usage after each iteration? I would say it should and casting my mind back to other services I've written they generally did - but I've been asleep since then and can't say with certainty. I get what you're saying about the AppSettingsReader ... definitely bad form to call it everytime anyway - even if it isn't the source of the leak. I spent an hour going through our company "data layer" yesterday - apparently the author was of the "the garbage collector will sort it out" type so nothing is ever disposed or closed or set to null. How our database server is still up I'll never know. So once I make sure my service code is all enclosed in a nice code nappy I'll have the joy of fixing that up - yay. Thanks for taking a look for me :)
-
In your OnStop method, you should Dispose of the timer rather than set it to null. As far as the rest goes, the garbage collector will run when it needs to. You shouldn't need to get the log filename every time, get it once and store it in a static field.
"WPF has many lovers. It's a veritable porn star!" - Josh Smith
As Braveheart once said, "You can take our freedom but you'll never take our Hobnobs!" - Martin Hughes.
-
I have a windows service in C# .net 2.0. At the moment all it is doing is being instantiated, setting up a timer and then every interval (eg 1 minute) writing to a log file. The memory usage on TaskManager goes up and up ... I can't see any objects that I've left lying around - I've taken out pretty much everything it is meant to do ... perhaps I'm being paranoid and the garbage collector will come and clean up when it is ready? How long should I leave it? It's been a while since the last Windows Service I wrote ... any ideas? Thanks in advance :) Here are some excerpts from the code... The LogFile is a static class with a single static method "Write"
public static class LogFile { public static void Write(string message) { string logFileName = (String)(new AppSettingsReader()).GetValue("log-file-path", typeof(String)) + "FileLoaderServiceLog.txt"; File.AppendAllText(logFileName, DateTime.Now.ToString("dd-MMM-yyyy HH:mm:ss") + " FileLoaderService:" + " " + message + Environment.NewLine); }
The Service class itself
public partial class FileLoaderService : ServiceBase { private Timer serviceTimer; public FileLoaderService() { InitializeComponent(); serviceTimer = new Timer(); double intervalInMinutes = (double)(new AppSettingsReader()).GetValue("timer-interval-in-minutes", typeof(double)); serviceTimer.Interval = intervalInMinutes \* 60 \* 1000; //intervalInMinutes \* 60 seconds \* 1000ms serviceTimer.Elapsed += new ElapsedEventHandler(serviceTimer\_Elapsed); } protected override void OnStart(string\[\] args) { LogFile.Write("Service Started - starting timer"); serviceTimer.Start(); } protected void serviceTimer\_Elapsed(object sender, ElapsedEventArgs e) { //stop timer whilst we process serviceTimer.Stop(); //do processing //have removed this to try to find the "leak" LogFile.Write("Checking Queue..."); //processing done - start it up again serviceTimer.Start(); } protected override void OnStop() { serviceTimer.Stop(); serviceTimer = null; LogFile.Write("Service Stopped"); } }
And Program Main ...
static class Program {
I implemented the suggestions given and the memory still grew. In the end the only way to stop the memory growth was to take out the logging altogether! So I had a service sit there and when the timer fired go Timer.Stop then Timer.Start - doing nothing else at all. Out of curiosity I changed the LogFile so it was no longer static and provided Open, Write & Close methods. In the constructor I grab the log file name from the config. On service start I call "Open" and on service Stop - "Close" and write to the log file at various points using Write. I don't like this idea for a multitude of reasons - but I did it just for testing purposes. I also removed any extraneous strings & DateTimes so my logFile.Write just writes the message to the file - no pretty DateTime stamp or anything. I've also tried a couple of variations of the actual write ... one that uses File.AppendAllText, one that uses a FileStream & StreamWriter and one that uses TextWriter with StreamWriter. Making sure I close and dispose appropriately. The File.AppendAllText was what I used in my static LogFile ... it seems to be the worst offender for memory usage. I've now got TextWriter along with the Open on start and Close on Stop. That seems like a pants way of achieving what I want to do. Now the memory consumption has slowed - but it still increases. Am I delusional in thinking that I can get this service to hover around a consistent memory usage? I left my original code (including calls to the database etc) running overnight and it got to 52Mb before I killed it. So in the hours between say 4pm yesterday afternoon and 7:00 this morning it grew from 8Mb to 50Mb. Not happy Jan :( My code needs a nappy and I have no idea where to go now :confused: I'm not comfortable installing this on a server for it to run 24 hours a day until it kills the server ... not sure I'd be too popular if I did that :)