Click here to Skip to main content
15,077,474 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
I made a class recently that implements IDisposable bacause it has a thread polling all the time a blocking condition.
I made a Dispose() method that in case it runs it unblocks the Thread but
unfortunately i am forced to call the Dispose method myself on FormClosed because for some reason it is not called by the GC and the project never actually closes.

Do you please have anything to suggest?

Edit: The class implements IPC through System.IO.Pipes with NamedPipes and unfortunately i just found out it is not yet supported from Mono.
If by any chance you can suggest any IPC full managed implementation i would appreciate it. I know that i could make the ServerNamedPipe Async but it requires a Win32Handle and therefore i went along with the standard implementation just in case it is included in Mono at some point.
Posted
Updated 4-Mar-10 1:51am
v2

Ok, looking at your code, it doesn't look like you implemented IDisposable correctly. You also didn't implement your singleton quite correctly. Lastly (this one doesn't effect functionality) While your OnMessage uses a delegate that takes a string array, it should be an actual object that inherits from EventArgs.

So I reimplemented the code try this first an EventArgs argument

C#
internal class EnumerableStringEventArgs : EventArgs
 {
     private readonly IEnumerable<string> _strings = null;
     public EnumerableStringEventArgs(IEnumerable<string> strings)
     {
         _strings = strings;
     }
     public IEnumerable<string> Strings
     {
         get { return _strings; }
     }
 }



Next your class implementing a singleton pattern, making the constructor private (eliminating the isfirst stuff) so I can force the use of Instance. I also used the EventHandler generic to make the event (eliminating the need for a delegate). I also implemented IDisposable and an IsDisposed property, and rather than using a boolean I used an int and the Interlocked functions to keep everything thread safe. I also changed the Thread to a threadpool thread, and changed the waiting mechaninsm to a ManualResetEvent, thus eliminating the need to keep track of the separate thread.

Finally I added a bunch of Debug.WriteLines so you could see it working.

C#
public sealed class SingleInstance : IDisposable
{
    private readonly ManualResetEvent _event = new ManualResetEvent(false);
    private readonly string _pipeName;
    private static readonly object _SyncLock = new object();
    private static SingleInstance _instance;
    private Int32 _disposed;
    private SingleInstance()
    {
        //private constructor, to prevent instantiation directly.
        _pipeName = string.Format("LocalPipe\\{0}",
                      Assembly.GetExecutingAssembly().GetName().Name);
        _event.Reset();
        ThreadPool.QueueUserWorkItem(RunServer);
    }
    ~SingleInstance()
    {
        Dispose(false);
    }
    public event EventHandler<EnumerableStringEventArgs> OnMessage;
    public static SingleInstance Instance
    {
        get
        {
            lock (_SyncLock)
            {
                if (_instance == null || _instance.IsDisposed)
                {
                    _instance = new SingleInstance();
                }
                return _instance;
            }
        }
    }
    public bool IsDisposed
    {
        get
        {
            return (Interlocked.Add(ref _disposed, 0) > 0);
        }
    }
    public String Path
    {
        get { return _pipeName; }
    }
    public void Dispose()
    {
        Dispose(true);
    }
    public void WaitOne(int timeoutMilliseconds)
    {
        _event.WaitOne(timeoutMilliseconds);
    }
    private void RunServer(object state)
    {
        try
        {
            NamedPipeServerStream pipeStream = null;
            while (!IsDisposed)
            {
                try
                {
                    pipeStream = new NamedPipeServerStream(_pipeName);
                    pipeStream.WaitForConnection();
                }
                catch
                {
                    pipeStream = null;
                    return;
                }
                var strings = new List<string>();
                using (var sr = new StreamReader(pipeStream))
                {
                    while (!sr.EndOfStream)
                    {
                        strings.Add(sr.ReadLine());
                    }
                }
                if (OnMessage != null)
                {
                    OnMessage(this, new EnumerableStringEventArgs(strings));
                }
            }
        }
        catch (Exception ex)
        {
            Debug.WriteLine(ex);
        }
        finally
        {
            _event.Set();
            //Debug.WriteLine("runserver finished.");
        }
    }
    private void Dispose(bool disposing)
    {
        try
        {
            Debug.WriteLine("Dispose(" + disposing + ")");
            if (Interlocked.Add(ref _disposed, 1) == 1)
            {
                if (disposing)
                {
                    GC.SuppressFinalize(this);
                }
                CloseServerSocket();
            }
        }
        finally
        {
            //Debug.WriteLine("Waiting for runserver to finish.");
            _event.WaitOne();
            Interlocked.Exchange(ref _disposed, 0);
            // Debug.WriteLine("Disposed.");
        }
    }
    private void CloseServerSocket()
    {
        try
        {
            using (var other = new NamedPipeClientStream(_pipeName))
            {
                other.Connect();
                using (var sw = new StreamWriter(other) { AutoFlush = true })
                {
                    sw.WriteLine("Shutdown");
                }
            }
        }
        catch
        {
            //eat the error quietly
        }
        finally
        {
            //Debug.WriteLine("Close socket finished.");
        }
    }
}



Finally a working test for it.

C#
public class Program
  {
      private static readonly ManualResetEvent _event = new ManualResetEvent(false);
      public static void Main(string[] args)
      {
          _event.Reset();
          using (var instance = SingleInstance.Instance)
          {
              instance.OnMessage += OnMessage;
              ThreadPool.QueueUserWorkItem(ThreadFunc);
              _event.WaitOne();
              instance.OnMessage -= OnMessage;
          }
          Debug.WriteLine("done");
      }
      private static void ThreadFunc(object state)
      {
          try
          {
              var bytes = Encoding.UTF8.GetBytes("test connection");
              var length = bytes.Length;
              for (int i = 0; i < 2; i++)
              {
                  using (var client = new NamedPipeClientStream(SingleInstance.Instance.Path))
                  {
                      client.Connect();
                      client.Write(bytes, 0, length);
                  }
              }
          }
          catch
          {
              //eat error quietly
          }
          finally
          {
              _event.Set();
          }
      }
      private static void OnMessage(object sender, EnumerableStringEventArgs e)
      {
          foreach (var s in e.Strings)
          {
              Debug.WriteLine(s);
          }
      }
  }
   
It sounds like one of the items isn't being closed, without seeing the code, its hard to tell what it might be.

GC.WaitForPendingFinalizers might help you though.
   
Garbage Collection

Read that link, will help you understand better the GC, but in a quick note, you can't control when the GC is going to be executed, but about your problem, show us some code to help you
   
Ok here it goes...

C#
public SingleInstance(bool IsFirst)
{
    pipeName = "LocalPipe\\" + Assembly.GetExecutingAssembly().GetName().Name;
    isFirst = IsFirst;
    if (isFirst)
    {
        WorkerProc = new Thread(new ThreadStart(RunServer));
        WorkerProc.Start();
        return;
    }
}

        void RunServer()
        {
            while (!disposing)
            {
                try
                {
                    pipeStream = new NamedPipeServerStream(pipeName);
                    (pipeStream as NamedPipeServerStream).WaitForConnection();
                }
                catch { return; }
                StreamReader sr = new StreamReader(pipeStream);
                List<String> strings = new List<string>();
                string temp;
                while ((temp = sr.ReadLine()) != null)
                    strings.Add(temp);
                sr.Close();
                if (OnMessage != null)
                    OnMessage.Invoke(strings.ToArray());
            }
        }

        public void Dispose()
        {
            if (!disposing)
            {
                disposing = true;
                if (isFirst)
                {
                    NamedPipeClientStream other;
                    other = new NamedPipeClientStream(pipeName);
                    other.Connect();
                    StreamWriter sw = new StreamWriter(other);
                    sw.AutoFlush = true;
                    sw.Write("Shutdown" + Environment.NewLine);
                    sw.Close();
                    WorkerProc.Join();
                }
            }
        }


As you can see its kind of lame the way i use now to make the thread continue but it works ;)

Edit: (pipeStream as NamedPipeServerStream).BeginWaitForConnection...
requires using Win32Handle and therefore i dont want to use it.
I mean that it throws exception if the Pipe is not of type Async and the Constructor that defines Async also requires Win32Handle
   
v2

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)




CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900