|
So this was a SQL question all along?
|
|
|
|
|
I have a manager class that is responsible for controlling a whole bunch of ports. Each port can only exist once so it's constructor is internal and used by the manager class only. This is a class library so I have no control how it will be used in any consuming application. It is critical that every port is closed at the end of the application so I have provided a CloseAll method. I can implement IDisposable but if Dispose or CloseAll isn't called it could have severe consequences that I'd like to avoid instead of saying you MUST call one of these methods.
The obvious solution is to use the destructor to call CloseAll or Dispose . I have experimented with various implementations using a static manager class, making the manager class a Singleton etc but none were reliable, I assume because the order of destruction of objects cannot be predicted, so CloseAll could be trying to close port instances that have already been GCed.
The sketch code below however seems to work 100% reliably (I have tried this in the full 'real' code with no problems too) and the destructor always gets called - and before any ports are collected. Perfect - but what I can't work out is why! Can anyone shed any light?
using System.Collections.Generic;
class Program
{
static void Main(string[] args)
{
Manager.Ports[0].Open();
}
}
public class Manager
{
private static List<Port> ports;
private static Manager manager = new Manager();
static Manager()
{
Initialize();
}
private Manager()
{ }
~Manager()
{
CloseAll();
}
public static IList<Port> Ports
{
get { return ports.AsReadOnly(); }
}
public static void CloseAll()
{
foreach (Port port in ports)
port.Close();
}
private static void Initialize()
{
ports = new List<Port>(1);
ports.Add(new Port());
}
}
public class Port
{
internal Port() { }
public void Close()
{
}
public void Open()
{
}
}
|
|
|
|
|
One way to handle this would be to add a Closed event to your Port class, and handle it in the manager. When you receive a Closed event, remove the relevant item from your list of ports.
"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.
My blog | My articles | MoXAML PowerToys | Onyx
|
|
|
|
|
I'm already doing that in the real code actually Pete, the issue was if Close wasn't called either on the individual Port s or on the Manager I needed to do it automatically before the Port instances are collected.
My code works, but I don't understand why. For some reason, the static manager I have created inside the Manager class always has it's destructor called before any other objects in there. Exactly what I need, but I'd like to understand why before relying on the behaviour.
|
|
|
|
|
Probably because you've got strong references in there, and there's nothing to release them until you have dereferenced them in the Manager class.
"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.
My blog | My articles | MoXAML PowerToys | Onyx
|
|
|
|
|
I don't think this is quite it. He never does 'dereference' them in the Manager class. AFAIK, the GC doesn't say that there are no references to the object, just that there are no reference chains back to one of the "root" objects. That is why the lack of strict ordering of finalizers is such a big deal; it is entirely possible that they could clean up the Ports first.
|
|
|
|
|
Gideon Engelberth wrote: it is entirely possible that they could clean up the Ports first
That's what I thought would be the case, but in practice it seems not. Somehow, creating an instance of the Manager class within the class seems to force finalization of that before any other references it may hold, which means I can always access them from the Manager 's destructor.
|
|
|
|
|
Hmmm... possibly. The fact that they are static probably creates that situation, but I'm struggling to work out why the private static manager instance I create always gets it's destructor called first. It must be to do with the fact that it resides within the manager class itself so somehow it prioritises that
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
You can add another quote to your sig:
"My code works, but I don't understand why." - DaveyM69
made me chuckle anyways.
...cmk
The idea that I can be presented with a problem, set out to logically solve it with the tools at hand, and wind up with a program that could not be legally used because someone else followed the same logical steps some years ago and filed for a patent on it is horrifying.
- John Carmack
|
|
|
|
|
It's the first time I've found myself in this situation - honest!
|
|
|
|
|
Done!
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
I'd say you are getting lucky.
It seems to me that you have the finalizer in the wrong place. The Manager does not directly own any unmanaged resources. Since the Port class does directly own the unmanaged resources (from the way you describe it), it should be Disposable and have a finalizer that calls Dispose(false) as per the Dispose pattern. Then it does not matter what order things get finalized in by the GC. Port would look something like this:
public class Port : IDisposable
{
~Port()
{
Dispose(false);
}
public void IDisposable.Dispose()
{
Dispose(true);
GC.SupressFinalize(this);
}
private bool alreadyDisposed;
protected void Dispose(bool disposing)
{
if (!alreadyDisposed)
{
if (disposing)
{
}
alreadyDisposed = true;
}
}
public void Close()
{
Dispose();
}
}
|
|
|
|
|
There are no unmanaged resources held by any class (although the Port class does call unmanaged functions). Each port must be reusable and when called from elsewhere, the same object must be retrieved so unfortunately implementing the dispose pattern doesn't solve the issue I was faced with.
Thanks anyway
|
|
|
|
|
Hi Dave,
First of all I'm no expert on destruction; I've read several articles about it, and find it difficult to grasp.
My initial comments on your code are:
1. I thought one wasn't supposed to write finalizers/destructors; instead one should provide a Dispose(bool) override.
2. I am pretty sure the GC does not finalize anything when an application exits; it does when things got collected, which only happens when a new need for memory cannot be solved without it;
3. The one Manager object you create sits in a static member of class Manager, so it never dies.
4. both (2) and (3) seen sufficient to say: I can't believe CloseAll() gets called automatically.
5. There is an Application.Exit event which I believe should be the key to solving your problem.
Some more thoughts:
6. The ports keep adding to your list, even when they got closed. And they may appear more than once.
7. I would avoid managers (I generally do!) and add overall port functionality to the Port class itself, using statics (as you have in Manager anyway).
In summary:
(A) I would fire your Manager and use Application.Exit to execute Port.CloseAll()
(B) I wouldn't say "I don't know why it works", my feelings are "I am surprised it would work as is".
Cheers.
Luc Pattyn
I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages
|
|
|
|
|
Luc Pattyn wrote: one should provide a Dispose(bool) override.
I agree, but then in the finalizer we would call Dispose(false); which would execute the same code so no real difference.
Luc Pattyn wrote: GC does not finalize anything when an application exits; it does when things got collected, which only happens when a new need for memory
Again, I was under the same impression, but my isolated tests seem to imply that isn't necessarily the case as it has got called every time without fail.
Luc Pattyn wrote: The one Manager object you create sits in a static member of class Manager, so it never dies.
Again, that's what I assumed. With it, the finalizer gets called, without it doesn't
Luc Pattyn wrote: Application.Exit
As this is a class library that will be consumed elsewhere I have no access to the Application object so although it would be ideal it isn't an available solution (I'll double check this tonight).
Thanks for your input as always Luc. The comments above seem negative but they are not intended that way!
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
You're welcome. I think I will investigate this whole strange situation...
Luc Pattyn
I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages
|
|
|
|
|
If it helps at all - this is some demo code expanded from the listing I gave here earlier. The actual code is obviously far more involved but this should give you the idea.
Be careful when experimenting with this - if a port is left open and the application exits then it will probably freeze the app and to release the port will require a system reboot. That's why I'm concened about this as that is not a desirable situation. Note, the Input , Output , Port and Manager classes will be in a separate assembly so their internal constructors will not be accessible.
I apologise for the length of the code but I thought it may be hepful.
using System;
using System.Collections.Generic;
using System.Runtime.InteropServices;
using System.Windows.Forms;
namespace MIDI.Test
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
Load += new EventHandler(Form1_Load);
FormClosing += new FormClosingEventHandler(Form1_FormClosing);
}
void Form1_Load(object sender, EventArgs e)
{
foreach (Port port in Manager.Ports)
{
port.Closed += new EventHandler(port_Closed);
port.Opened += new EventHandler(port_Opened);
port.Open();
}
}
void Form1_FormClosing(object sender, FormClosingEventArgs e)
{
Manager.CloseAll();
}
void port_Closed(object sender, EventArgs e)
{
Port port = sender as Port;
if(port !=null)
Console.WriteLine("{0}[{1}] closed", port.Type, port.ID);
}
void port_Opened(object sender, EventArgs e)
{
Port port = sender as Port;
if (port != null)
Console.WriteLine("{0}[{1}] opened", port.Type, port.ID);
}
}
public class Manager
{
private static List<Input> inputs;
private static List<Output> outputs;
private static List<Port> ports;
private static Manager manager = new Manager();
static Manager()
{
Initialize();
}
private Manager()
{ }
~Manager()
{
CloseAll();
}
public static IList<Input> Inputs
{
get { return inputs.AsReadOnly(); }
}
public static IList<Output> Outputs
{
get { return outputs.AsReadOnly(); }
}
public static IList<Port> Ports
{
get { return ports.AsReadOnly(); }
}
public static void CloseAll()
{
foreach (Port port in ports)
port.Close();
}
private static void Initialize()
{
int inputCount = Interop.midiInGetNumDevs();
int outputCount = Interop.midiOutGetNumDevs();
ports = new List<Port>(inputCount + outputCount);
inputs = new List<Input>(inputCount);
if (inputCount > 0)
for (int i = 0; i < inputCount; i++)
{
inputs.Add(new Input(i));
ports.Add(inputs[i]);
}
outputs = new List<Output>(outputCount);
if (outputCount > 0)
for (int i = 0; i < outputCount; i++)
{
outputs.Add(new Output(i));
ports.Add(outputs[i]);
}
}
}
public class Input : Port
{
internal Input(int id)
: base(PortType.Input, id)
{ }
public override void Close()
{
if (handle != IntPtr.Zero)
{
Interop.midiInClose(handle);
handle = IntPtr.Zero;
}
}
public override void Open()
{
if (handle == IntPtr.Zero)
Interop.midiInOpen(
out handle, id, callback, id, Interop.CALLBACK_FUNCTION);
}
}
public class Output : Port
{
internal Output(int id)
: base(PortType.Output, id)
{ }
public override void Close()
{
if (handle != IntPtr.Zero)
{
Interop.midiOutClose(handle);
handle = IntPtr.Zero;
}
}
public override void Open()
{
if (handle == IntPtr.Zero)
Interop.midiOutOpen(
out handle, id, callback, id, Interop.CALLBACK_FUNCTION);
}
}
public abstract class Port
{
public event EventHandler Opened;
public event EventHandler Closed;
internal Interop.MidiProc callback;
protected IntPtr handle;
protected int id;
internal Port(PortType type, int id)
{
callback = OnCallback;
handle = IntPtr.Zero;
this.id = id;
Type = type;
}
public int ID
{
get { return id; }
}
public PortType Type
{
get;
private set;
}
public abstract void Close();
protected virtual void OnCallback(IntPtr hMidi, int wMsg, int dwInstance, int dwParam1, int dwParam2)
{
switch (wMsg)
{
case Interop.MIM_CLOSE:
case Interop.MOM_CLOSE:
OnClosed(EventArgs.Empty);
break;
case Interop.MIM_OPEN:
case Interop.MOM_OPEN:
OnOpened(EventArgs.Empty);
break;
}
}
protected virtual void OnClosed(EventArgs e)
{
EventHandler eh = Closed;
if (eh != null)
eh(this, e);
}
protected virtual void OnOpened(EventArgs e)
{
EventHandler eh = Opened;
if (eh != null)
eh(this, e);
}
public abstract void Open();
}
public enum PortType
{
Input = 0,
Output = 1
}
internal static class Interop
{
public const int CALLBACK_FUNCTION = 0x00030000;
public const int MIM_OPEN = 0x3C1;
public const int MIM_CLOSE = 0x3C2;
public const int MOM_OPEN = 0x3C7;
public const int MOM_CLOSE = 0x3C8;
public delegate void MidiProc(
IntPtr hMidi,
int wMsg,
int dwInstance,
int dwParam1,
int dwParam2);
[DllImport("winmm.dll")]
public static extern int midiInClose(
IntPtr hMidiIn
);
[DllImport("winmm.dll")]
public static extern int midiInGetNumDevs();
[DllImport("winmm.dll")]
public static extern int midiInOpen(
out IntPtr lphMidiIn,
int uDeviceID,
MidiProc dwCallback,
int dwCallbackInstance,
int dwFlags
);
[DllImport("winmm.dll")]
public static extern int midiOutClose(
IntPtr hmo
);
[DllImport("winmm.dll")]
public static extern int midiOutGetNumDevs();
[DllImport("winmm.dll")]
public static extern int midiOutOpen(
out IntPtr lphmo,
int uDeviceID,
MidiProc dwCallback,
int dwCallbackInstance,
int dwFlags
);
}
}
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
Hi Dave,
thanks, I will look into this one of these days.
I can already confirm it does compile.
Luc Pattyn
I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages
|
|
|
|
|
Hi Dave,
I have simplified your code and added lots of observation code; it still behaves basically as before.
here are preliminary ideas/observations (with insufficient proof):
1. when instantiating a class with a destructor, the object is added to the finalizer queue right away;
2. when the app exits, the destructor of all objects in the finalizer queue is (probably?) executed;
3. 1+2 explains how your CloseAll gets executed at all; it does not need a GC pass, no mark-and-sweep or whatever they use to locate dead objects: at the app exit, everything is considered dead, so simply finalizing everything still in the finalizer queue makes sense (an object finalized earlier would have been removed from the list already).
4. on app exit the objects, in particular the ports, are not collected (wouldn't make any sense as all memory is about to get freed anyway).
5. the order in the finalizer queue might be simply chronological, hence your Manager instance comes before your Port instances. And as long as your Manager hasn't been collected, neither have your ports, as your Manager is keeping a list of ports.
6. I'm with Gideon[^] when he says your destructor isn't in the right place; a destructor/finalizer/dispose should not rely on any other object, so giving each Port its own destructor would solve that, and make the above #5 irrelevant.
This article[^] looks very relevant and interesting; I haven't read it completely yet.
I may write a little article on the subject. I'll be back with more.
Luc Pattyn
I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages
|
|
|
|
|
All very interesting points Luc which make a lot of sense when explaining the observed behaviour.
Re point 6. Normally I would agree with you, however... having a Dispose method would imply that a port couldn't/shouldn't be reused after disposal. All ports need to be reusable from anywhere (hence the Manager class to retrieve the same instance from at any time) until they are definately finished with i.e. at application exit, therefore I feel that having a 'Dispose' method would be confusing and counter intuitive to the consumer of the library as i don't actually want to dispose of anything and nothing is being actually disposed. I just want the ports to be correctly closed. To this end I have provided a Close method for each port, and a helper CloseAll in the port Manager.
Any comments on this (and anything) will be most welcome.
I'm about to study that article right now
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
Hi Dave,
I'm not saying your Port should implement IDisposable.
What I (and Gideon) mean is you should provide a finalizer for Port, so it can close itself; destructors/finalizers/disposers are not allowed to rely on other objects, so a Manager closing all Ports upon finalization is against the rules. So it seems:
- Ports should finalize themselves;
- the Manager may be redundant (happens often), or just provides a CloseAll but no finalizer calling it.
I haven't tested this yet.
BTW: The way I understood it Joe's article tries to make clear finalizers aren't 100% trustworthly. For one, a process kill (e.g. thru Task Manager) will not close the ports, whatever code you provide.
Luc Pattyn
I only read code that is properly indented, and rendered in a non-proportional font; hint: use PRE tags in forum messages
|
|
|
|
|
Closing itself can be problematic (see post below) I believe due to the callback that happens from the MIDI driver back to the port instance once closure has finshed so post the Close method. The static use of the ports seems to keep them alive for at least long enough for the callback (untested so far). I have previously attempted a static delegate with no improvement. Also, there is no gurantee when the Port's finalizer will be called so the auto close may not happen when it needs to.
(All this is just a safety net against stupidity to cope with the possible case that a consumer doesn't explicitly call Close on any open ports or Manager.CloseAll.)
Luc Pattyn wrote: finalizers aren't 100% trustworthly ... Task Manager
I don't believe, using managed code at least, that there is anything I can do to deal with that situation so I have no choice but to live with it.
That was a great link by the way - thanks! Bookmarked for rereading several times until it all sinks in and for future reference.
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
After sleeping on it, I'm pretty sure that the callback is the source of the issues that prevent me from using the finalizer (either with or without Dispose) to close the port. It's clearer to see in code! Is there any reliable way to prolong the life of a delegate instance?
using System;
using System.Runtime.InteropServices;
public class Output
{
private const int CALLBACK_FUNCTION = 0x00030000;
private const int MOM_CLOSE = 0x3C8;
private const int MOM_OPEN = 0x3C7;
private delegate void MidiProc(
IntPtr hMidi,
int wMsg,
int dwInstance,
int dwParam1,
int dwParam2);
[DllImport("winmm.dll")]
private static extern int midiOutClose(
IntPtr hmo
);
[DllImport("winmm.dll")]
private static extern int midiOutOpen(
out IntPtr lphmo,
int uDeviceID,
MidiProc dwCallback,
int dwCallbackInstance,
int dwFlags
);
private MidiProc callback;
private IntPtr handle;
private int id;
public Output(int id)
{
callback = OnCallback;
handle = IntPtr.Zero;
this.id = id;
}
~Output()
{
Close();
}
public void Close()
{
if (handle != IntPtr.Zero)
{
midiOutClose(handle);
handle = IntPtr.Zero;
}
}
private void OnCallback(
IntPtr hMidi, int wMsg, int dwInstance, int dwParam1, int dwParam2)
{
}
public void Open()
{
if (handle == IntPtr.Zero)
midiOutOpen(
out handle, id, callback, id, CALLBACK_FUNCTION);
}
}
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
Further to my earlier reply, if I were to implement IDisposable the Port class would need to look something like this. I think this is messier but I do understand the reasoning for the recommendation for using Dispose .
using System;
namespace MIDI.NET.Ports
{
public abstract class Port : IDisposable
{
bool disposed;
~Port()
{
Dispose(false);
}
public virtual void Close()
{
if (disposed)
throw new ObjectDisposedException(
ToString(),
"This port has been disposed. If you wish to reuse it please use the ReCreate method.");
}
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
protected virtual void Dispose(bool disposing)
{
if (!disposed)
{
if (disposing)
{
}
Close();
disposed = true;
}
}
public void Recreate()
{
if (disposed)
{
GC.ReRegisterForFinalize(this);
disposed = false;
}
}
}
}
I haven't tested this yet, but I'm 99% certain this gave me problems when I tried a similar implementation some months ago. Sometimes ports never closed so the application hung (often invisibly) or it created system instability, sometimes I got Access Violation (or similar) errors. I never got to the bottom of the problem but I figured it was one of the following:
1. Close being called on the instance could not be guaranteed to be successful as fields of the class may no longer be accessible (a loose theory and quite possibly/likely wrong!)
2. When calling Open on the port a callback is created which lasts beyond the call to Close as the Close itself generates a callback. Once this callback is registered it cannot be unregistered (that's the way the API works). Calling Close during Dispose may be too late as the callback delegate (and it's target) may now be collected making the unmanaged callback invalid (quite probable although badly explained!)
Dave
"My code works, but I don't understand why!" - DaveyM69 (Me) BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn) Why are you using VB6? Do you hate yourself? (Christian Graus)
|
|
|
|
|
Dear all,
Could anyone please help me with the following problem. I have a MainForm in my application. I call a background worker thread to do some work. This thread never finishes, so it will always run in the background and will be closed when the application closes. This works perfect. Then I open a new form from my main thread and in that form I call another backgroundworker. As long as backgroundworker 2 is running, backgroundworker 1 becomes unresponsive. The MainForm stays responsive, the newly opened form stays responsive, but the two backgroundworkers don't seem to like each other and seems to be competing over resources.
* MainForm - backgroundworker 1
* subForm - backgroundworker 2
When backgroundworker 2 is running, backgroundworker 1 becomes unresponsive until backgroundworker 2 is finished.
Weird huh ?
Kind regards,
|
|
|
|
|