Skip to content

Commit

Permalink
Fix crash at System.Net.Sockets.Socket.ResetHandle()
Browse files Browse the repository at this point in the history
  • Loading branch information
Fanglin Liu committed Mar 16, 2015
1 parent 70a02fb commit e461226
Showing 1 changed file with 53 additions and 0 deletions.
53 changes: 53 additions & 0 deletions Core/TcpClientSession.cs
Expand Up @@ -91,6 +91,12 @@ public SocketClientAccessPolicyProtocol ClientAccessPolicyProtocol
protected abstract void SocketEventArgsCompleted(object sender, SocketAsyncEventArgs e);

public override void Connect()
{
//ConnectUsingSocketStaticMethod();
ConnectUsingSocketInstanceMethod();
}

public void ConnectUsingSocketStaticMethod()
{
if (m_InConnecting)
throw new Exception("The socket is connecting, cannot connect again!");
Expand All @@ -117,6 +123,53 @@ public override void Connect()
#endif
}

protected void ProcessConnect2(object socket, SocketAsyncEventArgs e)
{
if (e != null && e.SocketError != SocketError.Success)
{
m_InConnecting = false;
OnError(new SocketException((int)e.SocketError));
return;
}

if (socket == null)
{
m_InConnecting = false;
OnError(new SocketException((int)SocketError.ConnectionAborted));
return;
}

if (e == null)
e = new SocketAsyncEventArgs();

e.Completed += SocketEventArgsCompleted;

Client = socket as Socket;

m_InConnecting = false;

#if !SILVERLIGHT
//Set keep alive
Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.KeepAlive, true);
#endif
OnGetSocket(e);
}

public void ConnectUsingSocketInstanceMethod()
{
string result = string.Empty;

Client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

SocketAsyncEventArgs socketEventArg = new SocketAsyncEventArgs();
socketEventArg.RemoteEndPoint = RemoteEndPoint;

socketEventArg.Completed += new EventHandler<SocketAsyncEventArgs>(ProcessConnect2);

Client.ConnectAsync(socketEventArg);

}

void Proxy_Completed(object sender, ProxyEventArgs e)
{
Proxy.Completed -= new EventHandler<ProxyEventArgs>(Proxy_Completed);
Expand Down

4 comments on commit e461226

@kerryjiang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your fix! But you should do more adjustment work to align with my original design.
It will be helpful to avoid introducing more other issues.
For example you change doesn't support IPv6 very well because you define the socket instance address family as AddressFamily.InterNetwork (IPv4 only) explicity.

Client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);

There must be other small isssues beside this one. But anyway, it is a great progress. Come on please, I am exiting to see somebody else can join me to develop good open source projects.

@fanglinliu
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you pointing out the issues. I am new C# developer for 2 months. I am iOS developer. There must be some other issues. Thanks again for pointing out this issue. I will improve that.

@codeknox
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there guys, any news on how should be the final code here?
So far, I've tested on WP, iOS and Droid and no error.
But my mobile app does not use ipv6 anyway, so looks like I'm safe.

@kerryjiang
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fanglinliu Do you have any update? Probably I need pay some time to understand how this change fix the issue.

Please sign in to comment.