3

I have a SocketState object which I am using to wrap a buffer and a socket together and pass it along for use with the Begin/End async socket methods. In the destructor, I have this:

~SocketState()
{
    if (!_socket.Connected) return;

    _socket.Shutdown(SocketShutdown.Both);
    _socket.Close();
}

When it gets to Close(), I get an ObjectDisposed exception. If I comment out the call Shutdown(), I do not get the error when it gets to the Close() method. What am I doing wrong?

Edit:

I know that the IDisposable solution seems to be how my code should be laid out, but that doesn't actually solve my problem. It's not like the destructor is getting called twice, so why would calling dispose() instead of using the destructor help me? I still get the same exception when calling these 2 functions in succession.

I have looked at the source for a similar server, and all they do is wrap these 2 statements in a try block and swallow the exception. I'll do that if I have to because it seems harmless (we're closing it anyway), but I would like to avoid it if possible.

ryeguy
  • 65,519
  • 58
  • 198
  • 260

3 Answers3

4

Using reflector; it appears that Close() essentially just calls Dispose() on the Socket (it does a bit of logging either side). Looking at Shutdown() it calls ws2_32.dll!shutdown() on the socket handle, which is also called by Dispose(). What's likely happening is that it's trying to call ws2_32.dll!shutdown() twice on the same socket handle.

The answer in short is to just call Close().

Phil Price
  • 2,283
  • 20
  • 22
  • This sounds reasonable, but the MSDN documentation explicitly states you should call shutdown before close so it finishes sending/receiving data . – ryeguy Mar 07 '09 at 21:21
  • Interesting; this *may* be because you are doing the work in the Finalizer; I would suggest taking *Jeroen Landheer's* suggestion, implementing IDisposable is the way to do this elegantly. – Phil Price Mar 07 '09 at 21:36
2

_socket implements IDisposable, and when your finalizer runs, the socket is already disposed (or finalized for that matter).

You should implement IDisposable on your class and follow the dispose pattern.

Example:

public class SocketState : IDisposable
{
  Socket _socket;

  public SocketState()
  {
    _socket = new Socket();
  }

  public bool IsDisposed { get; private set; }

  public void SomeMethod()
  {
    if (IsDisposed)
      throw new ObjectDisposedException("SocketState");

    // Some other code
  }

  #region IDisposable Members

  public void Dispose()
  {
    Dispose(true);
    GC.SuppressFinalize(this);
  }

  protected virtual void Dispose(bool disposing)
  {
    if (!IsDisposed)
    {
      if (disposing)
      {
        if (_socket != null)
        {
          _socket.Close();
        }
      }

      // disposed unmanaged resources

      IsDisposed = true;
    }
  }

  ~SocketState()
  {
    Dispose(false);
  }

  #endregion
}
Jeroen Landheer
  • 9,160
  • 2
  • 36
  • 43
  • I still need to call Shutdown() to complete sending data (per MDSN documentation). The example you gave doesn't really help anything..no matter where I call the code from it gives the same error. – ryeguy Mar 08 '09 at 07:00
  • As you can see in Phil's post, disposing the socket should be enough. But if you really want to call Shutdown first, put the call before _socket.Close() and change _socket.Close in _socket.Dispose() – Jeroen Landheer Mar 08 '09 at 20:29
0

Your question is somewhat unclear. You say, that when

it gets to Close(), you get an ObjectDisposedException

But look, Close doesn't throw anything. It's the Shutdown that does throw ObjectDisposedException. That's probably why commenting Shutdown out helps - it's exactly where you get the exception.

My explanation is that your socket gets .Dispose()'d automatically (through it's own finalizer) before your main object gets finalized.

As it's said here:

The execution order of finalizers is non-deterministic—in other words, you can't rely on another object still being available within your finalizer.

Community
  • 1
  • 1
Gman
  • 1,781
  • 1
  • 23
  • 38