Bobris / Nowin

Owin Web Server in pure .Net
MIT License
537 stars 78 forks source link

dispose of opened socket #30

Closed jchannon closed 9 years ago

jchannon commented 9 years ago

seems to solve #28

Bobris commented 9 years ago

I know about this bug in Mono it does not correctly implement: _disconnectEvent.DisconnectReuseSocket = true;

On .Net it correctly reuse socket, so it saves some allocations and OS calls. I would accept it if you check for environment type and did it only on Mono.

I still hope that someone will implement correct and optimal Saea implementation for Linux.

jchannon commented 9 years ago

Do you have any #if mono directives already setup?

If not would you be happy if I added them and would I set _disconnectEvent.DisconnectReuseSocket = false; as well as the socket.Dispose() already in this PR?

Bobris commented 9 years ago

I though about not doing it by #if directives but by checking "something" at runtime, so you don't need to recompile .dll you get from Nuget.

 public static bool IsRunningOnMono ()
 {
      return Type.GetType ("Mono.Runtime") != null;
 }
jchannon commented 9 years ago

Sure I can add that.

Do I just need to set that boolean value to false and dispose of the socket after that?

On 13 March 2015 at 07:22, Boris Letocha notifications@github.com wrote:

I though about not doing it by #if directives but by checking "something" at runtime, so you don't need to recompile .dll you get from Nuget.

public static bool IsRunningOnMono () { return Type.GetType ("Mono.Runtime") != null; }

Reply to this email directly or view it on GitHub https://github.com/Bobris/Nowin/pull/30#issuecomment-78845966.

Bobris commented 9 years ago

Set all 3:

        _receiveEvent.DisconnectReuseSocket = true;
        _sendEvent.DisconnectReuseSocket = true;
        _disconnectEvent.DisconnectReuseSocket = true;

To false already in void RecreateSaeas(), based on running on Mono. Maybe even name it "does not support DisconnectReuseSocket"

jchannon commented 9 years ago

Thanks. I've updated the PR

jchannon commented 9 years ago

Looks like as the PR is closed you cant see the changes in the diff

Bobris commented 9 years ago

Reopened, but still don't see anything.

jchannon commented 9 years ago

Should be there now :smile:

Bobris commented 9 years ago

I will polish it a little bit. And build new version after that.

jchannon commented 9 years ago

Thanks, appreciate it!

Bobris commented 9 years ago

Done, please test.

jchannon commented 9 years ago

Left an update on #28