When and where to call the RemoveHandler in VB.NET?
Asked Answered
V

6

2

I am working to a VB.NET windows forms projet in .NET 1.1. And I have this type of architecture, very simplified.

Public MustInherit Class BaseTestLogic

  Private _TimerPoll As Timer

  Public Sub New(ByVal sym As SymbolFileMng, ByVal cfg As LampTestConfig, ByVal daas As DaasManager, ByVal mcf As Elux.Wg.Lpd.MCFs.VMCF)

    AddHandler _TimerPoll.Tick, AddressOf TimerPoll_Tick

  End Sub

End Class

Public Class SpecificTestLogic
  Inherits BaseTestLogic      

End Class

Depending of the type of test I am doing I create an instance of a specific test derived from BaseTestLogic. But I found that after hundreds of object creations I can have StackOverflow exception.

I checked my code and saw that I forgot to remove the handler to Timer Tick. The question is, where and when is it correct to remove hadler?

Do I need to implement the IDisposable interface in the base class and RemoveHandler in Dispose?

Vorous answered 27/4, 2010 at 12:45 Comment(0)
I
1

You may go along with removing the handler when the Dispose is called, but a purist would say that "you shouldn't abuse IDisposable for purposes other than disposing unmanaged resources".

Another option is to remove the handler at the Finalize method.

You can also feel comfortable about removing the handler at several different places, if that makes any sense in your design. Removing an already removed handler will not cause any issue - unless the event is a Custom Event and its AddHandler/RemoveHandler implementations don't match the behavior of non-custom events (which is simply to use [Delegate].CombineDelegate/[Delegate].Remove). Just don't tell your purist friends about it; they won't comply.

Inchmeal answered 27/4, 2010 at 13:28 Comment(16)
It is strange but in my case the finalizer is never called, what could be the reason?Vorous
The reason is probaby because some reachable object still holds a reference to the BaseTestLogic object or one of its members (event handlers included). An object is finalized only after there are no (strong) references to it (and the garbage collector decides it needs to remove the object).Inchmeal
At the end I implemented the Disposable pattern and RemoveHandler to Tick event in the Dispose. The problem seems solved now. I am not a purist nor, luckily, colleagues that work with me :DVorous
what M.A. Hainn is getting at, but not quite saying directly, is that if the Finalizer isn't being called you may already have a resource-leak (essentially the managed-version of a memory-leak). Tracking these down can be tricky and using a profiler can greatly assist, but they can seem like ghostly issues until you get a feel for what causes objects to live longer than they should.Prepossess
Unsubscribing from events in the Finalize method of an object that receives them is useless. An event subscriber will always be kept alive as long as the event publisher (unless it unsubscribes). If the object is finalized, it will because the event publisher itself went out of scope, by which point unsubscription would become moot. Further, any "purist" who says one shouldn't unsubscribe events in Dispose is wrong. EVENTS ARE UNMANAGED RESOURCES. A managed resource is one that can be taken care of via finalize. Since events cannot, they are unmanaged.Misshapen
@Misshapen EVENTS ARE UNMANAGED RESOURCES is loud mistake. The problem is IDisposable Timer object wich would lead to implement a Dispose invocation on managed _TimerPoll private member. At most this forces us to consider BaseTestLogic IDisposable too.Expansive
@rfb: A publicly-exposed class which subscribes to timer events should implement IDisposable unless all instances are intended to remain useful throughout the lifetime of the program, or unless it will unsubscribe itself in reasonably timely fashion when the timer events fire. The GC won't be able to finalize an object with a live timer subscription, and if the application doesn't tell ` BaseTestLogic` its services are no longer needed, it would have no way of ever finding out and relaying the message to the timer whose event it subscribed.Misshapen
@rfb: A "resource" is essentially a responsibility to ensure that some action will get performed once it would no longer interfere with anything else; the most common such action is to notify some form of entity that its services are no longer required. The proper function of Dispose is not "destroy an object", but rather to deliver the message "Your services are no longer required. This is likely your final notice. Make of that what you will." This among other things lets objects discharge any responsibilities they have to tell other entities their services are no longer required.Misshapen
@Misshapen Sorry but I didn't understand your answer. Nothing personal but I'm trying to post a detailed answer as there seem to be different levels of misunderstandingExpansive
@rfb: What part do you not understand? I think the term "resource" gets used a lot without bothering to actually define it except by listing examples, but what all examples have in common is that "acquiring a resource" means taking on a responsibility to ensure that something gets cleaned up, and that to "release resources" one must discharge all associated responsibilities. A "managed resource" is one whose associated obligations can be carried out automatically with the help of Finalize, but events generally do not qualify.Misshapen
@Misshapen managed resource means CLR managed and GC collected on the managed heap with no need for custom resources/handles cleanup because there are no handles allocations (memory, file, db connections and so on) by external/raw/API and subsequent leakes in managed heap garbage collectionExpansive
@Misshapen there are some forms of unmanaged allocation in PIvoke usage / Com interopExpansive
@rfb: Unmanaged allocations are examples of resources. I haven't done much with .NET in many years, but everything I'd seen tended tended to be rather vague about what a "resource" was; if you know of any actual solid definitions, I'd be curious to see them. If an object can be created without incurring any associated responsibilities, such creation does not acquire any "resources", managed or otherwise. If creation of an object incurs responsibilities which cannot be carried out automatically via Finalize or other such means, such creation acquires unmanaged resources.Misshapen
@rfb: The question of whether a "resource" is "managed" is orthogonal to the nature of the associated responsibilities and whether they involve things that aren't managed objects. If a class uses an array of structures as a pool, with an AllocItem method that marks an "unused" element as "busy" and return its index, and a ReleaseItem method that marks a "busy" element at a given index as "unused", the act of calling AllocItem would acquire an unmanaged resource, even though it involves only managed objects, since failing to call ReleaseItem would render a slot permanently unusable.Misshapen
i'm using managed vs unmanaged in the .NET terminilogy senseExpansive
Let us continue this discussion in chat.Expansive
A
0

If you add them in the constructor it would feel right to remove them in the Dispose, but it of course depends on your design.

Here is a question with information about when you need to worry about removing them
AddHandler/RemoveHandler Not Disposing Correctly

Antepast answered 27/4, 2010 at 12:52 Comment(2)
I already read the linked question, but it didn't help much. I know that I should remove the handler but not sure where. Dispose could be an idea, I would like to be sure is the best practice.Vorous
Well one good reason to add them to Dispose in my view is that the user of your class can then use Using.Antepast
H
0

My first thought is that your actual problem has little, if anything, to do with adding and removing event handlers. A StackOverflowException means you have a series of functions that are creating an infinite loop of recursive calls. The code you posted does not show anywhere that could happen, but the stack trace of the exception should point you to the offending code.

Based on your comment about creating a test of a derived type, I wonder if you could post more of the code from the constructor in your base class.

Hellion answered 28/4, 2010 at 2:37 Comment(3)
@Gideon Engelberth: A StackOverflowException means that the stack is overflown. It is often because of an infinite loop / recursion, but that is simply because an infinite loop / recursion overflows the stack rapidly. Do note: they are not the same thing, and an infinite loop / recursion is not the meaning of a StackOverflow Exception.Inchmeal
@M.A. Hanin: Perhaps I should have said "usually means", but whenever I see a stack overflow exception, I'm going to look for recursion since its by far the most common reason for such an exception.Hellion
I think @GideonEngelberth is right because there must be some form of recursion in TimerPoll_Tick handler, due to floodness of method invocations by Timer wrappers kept alive for a refernce to a bigger lifetime object, or static reference, the one most likely hosting TimerPoll_Tick event handler.Expansive
Q
0

This is an old question but I came to it via web search so it is still a useful question about Timers. No appropriate answer was given to your UNDERLYING problem. Answers were offered to your question, but the question was wrong. Your question should be: How do I disable a timer so the event no longer fires?

Dim WithEvents timer1 as new Timer()

(then add timer1_Elapsed to your code)

This solves the problem of worrying about IDisposable or Finalize because the Timer event handler is managed for you. When you no longer need the timer, set Enabled = False or call the Stop() method to keep it from ticking.

Quince answered 25/3, 2015 at 14:27 Comment(1)
It's important to note that if the timer isn't disabled, anything to which it holds a reference will be kept alive forever whether or not it has any visibly useful effect. Disabling a timer and abandoning it will clean up events if no outside references exist to any of the objects interconnected by events, but it's probably better to set timer1 to Nothing (which will disconnect the event handlers) than hope they get disconnected.Misshapen
E
0

The decision of Timer Class designer to implement IDisposable must be considered as a statement that implementation deals with some form of referencing to non-.NET API / raw OS resource allocation (sorry for the non-canonical denomination).

As @supercat mentioned, an event subscriber will always be kept alive as long as the event publisher is, this leading to the problem of a reference to a bigger lifetime object, as concluded in In vb.net, if I use AddHandler, Do I have to use RemoveHandler?, and NOT because event handlers are unmanaged resources.

Event handlers are not "unmanaged resources" in the sense used on .NET terminilogy as opposed to "managed": no need of explicit allocation management through either Finalize or Dispose() for resource release and clean up.

From Troelsen 2010 Pro C# :

Finalizers can be used to release unmanaged resources when the garbage collector kicks in. However, given that many unmanaged objects are “precious items” (such as raw database or file handles), it may be valuable to release them as soon as possible instead of relying on a garbage collection to occur. As an alternative to overriding Finalize(), your class could implement the IDisposable interface, which defines a single method named Dispose().

ERRATA: see Is unwired event a memory leak? So, returing to the question, if TimerPoll_Tick event handler belongs to an bigger lifetime object, BaseTestLogic instances are never released. I don't know for sure but probably this would happen even if TimerPoll_Tick is a BaseTestLogic static method.

Returning to the question, it seems very strange the stackoverflow exception and could be more related to some form of design failure in TimerPoll_Tick event handling than to an object lifetime issue, like @GideonEngelberth suggested.

Expansive answered 27/1, 2021 at 19:14 Comment(0)
T
-1

It is strange but in my case the finalizer is never called, what could be the reason?

Or GC.SupressFinalize() has been called somewhere in your code?

Teressaterete answered 23/7, 2010 at 13:14 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.