1

I have a ASP.NET C# singleton (scoped to a session through HttpContext.Current.Session) which receives messages from code (warnings, errors, exceptions etc.). The messages are related to code problems, mainly used while debugging, not during production.

I wrote the custom destructor for this object so that its contents are written/appended as a file on a disk.

I wanted to ask two things related to this situation:

a] Is is a good idea to open a file and write to it during object destructor? The concurrency IO access is handled through static lock.

b] When is the destructor called for session scoped objects? Is it only when the session is expired on the server?

NeverStopLearning
  • 958
  • 11
  • 21
  • That just sounds like a weird, custom made solution. Investigate ELMAH (very little configuration)........and log4Net (some basic code changes.....but the "publishing" code is already created.....to published your messages to .. Sql Server to a Text File to Lots of other options.........already done. – granadaCoder Jul 18 '13 at 14:59
  • i think would be better in Dispose - http://msdn.microsoft.com/en-us/library/system.idisposable.aspx – terrybozzio Jul 18 '13 at 15:00
  • 1
    The destructor is in reality the `override void Finalize()` method. You have no control on when it will be called, the GC does it when the managed stack is full and it's next in his linked list. – Pierre-Luc Pineault Jul 18 '13 at 15:06
  • 1
    Sounds like you're storing all the messages in memory (`List`?) and *only* write them out when the session ends. Why not log immediately? – Corak Jul 18 '13 at 15:16
  • @Corak Yes, thanks, I have thought of that too as part of refactoring process and I will probably do it that way. You probably know how it usually is when you are developing whithout clear specs. You keep adding functionality because requirements change and then at some point, you need to stop and refactor the whole class. Previously we did not require this object to write to the disk log. – NeverStopLearning Jul 19 '13 at 10:16

2 Answers2

1

To accomplish this implement IDisposable and then wrap instances of your logger in a using block which will guarantee to call the dispose method on your logger.

Zache
  • 1,023
  • 6
  • 14
  • Is this applicable to the ASP.NET session-scoped singleton? I dont want the instance to go out of scope for the entire duration of the session. But maybe I just misunderstand the proposed solution as to what exactly to wrap inside a using block. – NeverStopLearning Jul 19 '13 at 10:09
1

I also recommend using some existing logging package. If you do decide to do this yourself, and just to bare in mind for the future:

a) No it's not a good idea. You shouldn't access managed resources in the finalizer (destructor), so if you have some log strings in memory for example, it's bad practice to access them (or the list they are contained in) as they may have already been finalized themselves at this point.

I don't want to repeat the recommended pattern, so see https://stackoverflow.com/a/1943856/2586804

You'll see there is only one place you should access managed during Dispose and this is if it is called by user code and not be the GC. So this should help you come to the conclusion that to achieve this, you must call .Dispose() yourself (or by using a using) as when (and if) the GC does it, it cannot access the managed members that contain the log lines.

b) Don't know, but it doesn't matter as you cannot use finalizer for this purpose anyway.

The bottom line is you can't rely on GC to run code for you. It's bad practice because you don't know when it's going to happen, plus any reference to the object anywhere, now or in the future will prevent the object being collected and introduce a bug.

You also shouldn't get c# Finalizers/Destructors to run code because that's not what they are for, they are for freeing unmanaged resources so that the machine doesn't run out. And note, it's a rare occurrence to use them in C# because most peoples day-to-day work is all with managed objects.

Instead explicitly tell the object to write it's logs, a method called Flush would be a good name. Or just let it write one line at a time. This would be the usual behaviour.

Community
  • 1
  • 1
user2586804
  • 321
  • 1
  • 10
  • Thank you for the answer. I am not going to used an existing logging package, mainly because I want to learn how things work (such as in this case). That being said, the List that I use for storing might already be collected when I am inside a class destructor? That goes against what I though GC does - collect unreferenced objects. Any hint on question (b)? Also thanks for the link, I will read it asap. – NeverStopLearning Jul 19 '13 at 10:13
  • 1
    I suggest you read this answer http://stackoverflow.com/a/538238/2586804: Pay particular attention to: "_It is entirely possible that in your Dispose() code, the managed object you're trying to get rid of (because you wanted to be helpful) is no longer there_" and "_So what you need is a way for Finalize() to tell Dispose() that it should not touch any managed resources (because they might not be there anymore)_" – user2586804 Jul 19 '13 at 12:13
  • Also for b) It's irrelevant, that is it doesn't matter when the finalizer is called as you won't be able to use the finalizer for your purpose anyway. – user2586804 Jul 19 '13 at 12:15
  • Thanks. It might be irrelevant for this case, but I was just curious in general. – NeverStopLearning Jul 21 '13 at 11:12