6

I want something very simple

// increment counter
Interlocked.Increment(ref _counter);

// automatically decrement counter after 1 sec
Timer timer = new Timer((o) => {
    Interlocked.Decrement(ref _counter);
    (o as Timer).Dispose();
}, timer, 1000, Timeout.Infinite);

this code, however, is not compile-able

Use of unassigned local variable 'timer'

Any easy way to fix that? It must be Threading.Timer.

P.S.: I am not sure if I must call Dispose, it's obviously un-managed resource and it's IDisposable, still on msdn they warn

As long as you are using a Timer, you must keep a reference to it. As with any managed object, a Timer is subject to garbage collection when there are no references to it. The fact that a Timer is still active does not prevent it from being collected.

And I actually want it to be be collected (auto-disposed?). So, to dispose or to not dispose?

Community
  • 1
  • 1
Sinatr
  • 20,892
  • 15
  • 90
  • 319

3 Answers3

6

There's a pattern for that:

Timer timer = null;
timer = new Timer...

Now you can use timer in the lambda body. This is basically a way to make the compiler happy while doing the same thing.

Be sure to dispose of that timer. Timers might be backed up by an OS handle resource (the CLR implementation changed a few times I believe). A timer also contains a GC handle I believe. This is harmless not to dispose of most of the time. But who knows what rare resource exhaustion you can provoke with a high volume of undisposed timers.

Doing a little code review: If you expect a cast to always work, do not use as because as documents that failure is an expected case. Not: (o as Timer). Instead: ((Timer)o).

usr
  • 168,620
  • 35
  • 240
  • 369
  • God damn, it's like I'm a Java programmer. – Marcel N. Aug 11 '14 at 10:41
  • 2
    @MarcelN. nobody wants to feel like that. – usr Aug 11 '14 at 10:44
  • Take a look at the original comments. I suggested something similar but as others pointed out, a `NullReferenceException` will occur when you try to Dispose the timer :c Spot on otherwise. – User 12345678 Aug 11 '14 at 10:44
  • @ByteBlast don't use the state parameter (because you're right with that - doesn't work). Use the captured variable `timer`. – usr Aug 11 '14 at 10:44
  • @ByteBlast: usr means to use the timer variable directly in the lambda body, because it is accessible there. Not sure if you meant that. – Marcel N. Aug 11 '14 at 10:45
  • Ah I see. I think I got confused because you still talk about the state object in your code review. – User 12345678 Aug 11 '14 at 10:47
  • Thanks, it works now. *Tricking* compiler is the least thing I'd expect from `C#`. I also appreciate that code review part, reading [this](http://stackoverflow.com/q/7566212/1997232) reveals another my bad habit. You'd better include full example, with captured variable, I just got tricked until read comments =D – Sinatr Aug 11 '14 at 11:13
  • @Sinatr if you care to find out *why* we have to trick the compiler here post a new question. The rules make sense. This is one of the rare cases where they result in awkward code. – usr Aug 11 '14 at 11:51
  • A slightly better way to do this is to use an array to encapsulate the timer inside an array. Reusing a local variable inside a closure can lead to side effects in some compiler versions. `Timer[] timers = new { null }; timers[0] = new Timer...` – Dmitry S. Aug 17 '16 at 05:19
  • @DmitryS. what side-effects are there? I'm not aware of that. Instead of an array I'd recommend `Box`. – usr Aug 17 '16 at 11:32
  • Here is an explanation: http://stackoverflow.com/questions/5438307/detailed-explanation-of-variable-capture-in-closures . If you are using Resharper, it will show a warning. – Dmitry S. Aug 17 '16 at 13:44
  • @DmitryS. what's your point? Avoiding allocations? Your suggested workaround allocates as well and it does not even avoid the closure. – usr Aug 17 '16 at 14:23
3

Self disposing won't work if you don't keep a reference to it. You risk having it garbage collected, even if it's not periodical. So, it may be garbage collected before even it ticks the first time.

From the System.Threading.Timer documentation:

As long as you are using a Timer, you must keep a reference to it. As with any managed object, a Timer is subject to garbage collection when there are no references to it. The fact that a Timer is still active does not prevent it from being collected.

On the other hand, it will be released via it's finalizer even if you don't dispose it (although not a good practice to rely on this). This can be seen in the Timer's source, on ReferenceSource.

Marcel N.
  • 13,726
  • 5
  • 47
  • 72
  • I am not sure if that quote from the docs is accurate today. I believe a timer keeps a GC handle to itself to keep it alive. If @HansPassant is around he can surely comment because he knows timers well. – usr Aug 11 '14 at 10:42
  • @usr: It does indeed keep a reference to itself, through an internal TimerQueue class which actually does the finalizing. Will look better into it and update (if Hans does not comment in the interim). – Marcel N. Aug 11 '14 at 11:02
0

If it's important that you dispose, you should try this:

class Foo {
  private Timer _timer;
  ...

  public void IncrementForASecond()
  {
    Interlocked.Increment(ref _counter);
    if(_timer != null)
      _timer.Dispose();
    _timer = new Timer((o) => {
    Interlocked.Decrement(ref _counter);
    }, null, 1000, Timeout.Infinite);
  }

This way, you are making sure that you always have a reference to the timer and that you are disposing of the previous timer before creating a new one.

Adrian Deac
  • 11
  • 1