haskell-distributed / network-transport-tcp

TCP Realisation of Network.Transport
http://haskell-distributed.github.com
BSD 3-Clause "New" or "Revised" License
30 stars 27 forks source link

Fixes NTTCP-11. Makes send strict for self connections. #19

Closed facundominguez closed 9 years ago

facundominguez commented 9 years ago

https://cloud-haskell.atlassian.net/browse/NTTCP-11

edsko commented 9 years ago

Basically this change is making send strict for all connections, is that true?

facundominguez commented 9 years ago

send is strict for all connections to other endpoints. This patch is making it strict for the same endpoint as well.

I can think of two reasons to do it:

Before this change, one could crash the node controller of CH by calling:

DP.spawnAsync self (error "bang!")

which is going through the transport even for the local node.

If the sender gets the error, it is less puzzling.

qnikst commented 9 years ago

This is part of connectToSelf call that is called inside:

apiConnect params ourEndPoint theirAddress _reliability hints =
  try . asyncWhenCancelled close $
    if localAddress ourEndPoint == theirAddress
      then connectToSelf ourEndPoint

so I'd say that this patch do not affect non-local connections in any way.

edsko commented 9 years ago

Yes, that's what I meant: it was already strict when sending to other endpoints, this patch makes it strict for same-endpoint too. Makes sense, although Tim had some "unsafe" primitives that explicitly were NOT strict when sending stuff to the same endpoint. I don't know if that is affected by this patch or not.

qnikst commented 9 years ago

IIUC that part of functionality is not affected, because d-p do not use network-transport for local communication.

edsko commented 9 years ago

You may well be right, I don't remember :) But that sounds reasonable :) Mentioned it just in case.

qnikst commented 9 years ago

@edsko while you are here could you also check #18 ?

facundominguez commented 9 years ago

@qnikst pointed that evaluate is not contributing anything, so I shortened the patch.