Aren/haiku
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

* Made the endpoint manager wait on bind() if the existing connection is

Browse Source 
local, and is about to close.
* This fixes several race conditions with the neon test suite that relied
  on TCP sockets being gone after close, and their port being available again
  (note, that is not what the TCP spec says, anyway, but it makes sense to
  do so, since we already removed the time wait state for local connections).
* The endpoint manager is now using a mutex instead of a semaphore.
* TCPEndpoint::_Close() now notifies the senders/receivers instead of
  _HandleReset().
* Besides ssl_closure(), all tests of the neon test suite seem to pass now.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@25564 a95241bf-73f2-0310-859d-f6bbb57e9c96
This commit is contained in:
Axel Dörfler 2008-05-19 18:27:10 +00:00
parent 8eadf4d51f
commit 30a396ab42
5 changed files with 69 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
src/add-ons/kernel/network/protocols/tcp/EndpointManager.cpp
View File

@ -221,29 +221,24 @@ EndpointManager::EndpointManager(net_domain* domain)
fConnectionHash(this),
fLastPort(kFirstEphemeralPort)
{
benaphore_init(&fLock, "endpoint manager");
mutex_init(&fLock, "endpoint manager");
}
EndpointManager::~EndpointManager()
{
benaphore_destroy(&fLock);
mutex_destroy(&fLock);
}
status_t
EndpointManager::InitCheck() const
{
if (fConnectionHash.InitCheck() < B_OK)
return fConnectionHash.InitCheck();
status_t status = fConnectionHash.InitCheck();
if (status == B_OK)
status = fEndpointHash.InitCheck();
if (fEndpointHash.InitCheck() < B_OK)
return fEndpointHash.InitCheck();
if (fLock.sem < B_OK)
return fLock.sem;
return B_OK;
return status;
}
@ -266,7 +261,7 @@ EndpointManager::SetConnection(TCPEndpoint* endpoint, const sockaddr* _local,
{
TRACE(("EndpointManager::SetConnection(%p)\n", endpoint));
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
SocketAddressStorage local(AddressModule());
local.SetTo(_local);
@ -292,7 +287,7 @@ EndpointManager::SetConnection(TCPEndpoint* endpoint, const sockaddr* _local,
status_t
EndpointManager::SetPassive(TCPEndpoint* endpoint)
{
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
if (!endpoint->IsBound()) {
// if the socket is unbound first bind it to ephemeral
@ -319,7 +314,7 @@ EndpointManager::SetPassive(TCPEndpoint* endpoint)
TCPEndpoint*
EndpointManager::FindConnection(sockaddr* local, sockaddr* peer)
{
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
TCPEndpoint *endpoint = _LookupConnection(local, peer);
if (endpoint != NULL) {
@ -366,7 +361,7 @@ EndpointManager::Bind(TCPEndpoint* endpoint, const sockaddr* address)
// if (!AddressModule()->is_understandable(address))
// return EAFNOSUPPORT;
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
if (AddressModule()->get_port(address) == 0)
return _BindToEphemeral(endpoint, address);
@ -378,11 +373,12 @@ EndpointManager::Bind(TCPEndpoint* endpoint, const sockaddr* address)
status_t
EndpointManager::BindChild(TCPEndpoint* endpoint)
{
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
return _Bind(endpoint, *endpoint->LocalAddress());
}
/*! You must hold fLock when calling this method. */
status_t
EndpointManager::_BindToAddress(TCPEndpoint* endpoint, const sockaddr* _address)
{
@ -397,25 +393,52 @@ EndpointManager::_BindToAddress(TCPEndpoint* endpoint, const sockaddr* _address)
if (ntohs(port) <= kLastReservedPort && geteuid() != 0)
return B_PERMISSION_DENIED;
EndpointTable::ValueIterator portUsers = fEndpointHash.Lookup(port);
bool retrying = false;
int32 retry = 0;
do {
EndpointTable::ValueIterator portUsers = fEndpointHash.Lookup(port);
retry = false;
while (portUsers.HasNext()) {
TCPEndpoint *user = portUsers.Next();
while (portUsers.HasNext()) {
TCPEndpoint* user = portUsers.Next();
if (user->LocalAddress().IsEmpty(false)
|| address.EqualTo(*user->LocalAddress(), false)) {
if ((endpoint->socket->options & SO_REUSEADDR) == 0)
return EADDRINUSE;
// TODO lock endpoint before retriving state?
if (user->State() != TIME_WAIT && user->State() != CLOSED)
return EADDRINUSE;
if (user->LocalAddress().IsEmpty(false)
|| address.EqualTo(*user->LocalAddress(), false)) {
// Check if this belongs to a local connection
// Note, while we hold our lock, the endpoint cannot go away,
// it can only change its state - IsLocal() is safe to be used
// without having the endpoint locked.
tcp_state userState = user->State();
if (user->IsLocal()
&& (userState > ESTABLISHED || userState == CLOSED)) {
// This is a closing local connection - wait until it's
// gone away for real
mutex_unlock(&fLock);
snooze(10000);
mutex_lock(&fLock);
// TODO: make this better
if (!retrying) {
retrying = true;
retry = 5;
}
break;
}
if ((endpoint->socket->options & SO_REUSEADDR) == 0)
return EADDRINUSE;
if (userState != TIME_WAIT && userState != CLOSED)
return EADDRINUSE;
}
}
}
} while (retry-- > 0);
return _Bind(endpoint, *address);
}
/*! You must hold fLock when calling this method. */
status_t
EndpointManager::_BindToEphemeral(TCPEndpoint* endpoint,
const sockaddr* address)
@ -486,7 +509,7 @@ EndpointManager::Unbind(TCPEndpoint* endpoint)
return B_BAD_VALUE;
}
BenaphoreLocker _(fLock);
MutexLocker _(fLock);
if (!fEndpointHash.Remove(endpoint))
panic("bound endpoint %p not in hash!", endpoint);

2
src/add-ons/kernel/network/protocols/tcp/EndpointManager.h
View File

@ -106,7 +106,7 @@ private:
typedef OpenHashTable<ConnectionHashDefinition> ConnectionTable;
typedef MultiHashTable<EndpointHashDefinition> EndpointTable;
benaphore fLock;
mutex fLock;
net_domain* fDomain;
ConnectionTable fConnectionHash;
EndpointTable fEndpointHash;

45
src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.cpp
View File

@ -220,6 +220,7 @@ enum {
FLAG_NO_RECEIVE = 0x04,
FLAG_CLOSED = 0x08,
FLAG_DELETE_ON_CLOSE = 0x10,
FLAG_LOCAL = 0x20
};
@ -497,21 +498,6 @@ TCPEndpoint::Close()
TRACE("Close(): after waiting, the SendQ was left with %lu bytes.",
fSendQueue.Used());
#if 0
// TODO: For the following to work we also need to be notified when TIME_WAIT
// is entered.
} else if (fRoute != NULL && (fRoute->flags & RTF_LOCAL) != 0) {
// This is a local connection - just wait until we're done so that
// the port is available again
while (fState != CLOSED && fState != TIME_WAIT) {
if (socket->error != B_OK)
return socket->error;
status_t status = fSendList.Wait(locker);
if (status < B_OK)
return status;
}
#endif // 0
}
return B_OK;
}
@ -1022,6 +1008,13 @@ TCPEndpoint::IsBound() const
}
bool
TCPEndpoint::IsLocal() const
{
return (fFlags & FLAG_LOCAL) != 0;
}
status_t
TCPEndpoint::DelayedAcknowledge()
{
@ -1058,8 +1051,7 @@ TCPEndpoint::_EnterTimeWait()
_CancelConnectionTimers();
if (fState == TIME_WAIT && fRoute != NULL
&& (fRoute->flags & RTF_LOCAL) != 0) {
if (fState == TIME_WAIT && IsLocal()) {
// we do not use TIME_WAIT state for local connections
fFlags |= FLAG_DELETE_ON_CLOSE;
return;
@ -1131,13 +1123,9 @@ TCPEndpoint::_MarkEstablished()
status_t
TCPEndpoint::_WaitForEstablished(MutexLocker &locker, bigtime_t timeout)
{
#if 0
// TODO: Checking for CLOSED seems correct, but breaks several neon tests.
// When investigating this, also have a look at _Close() and _HandleReset().
while (fState < ESTABLISHED && fState != CLOSED) {
#else
while (fState < ESTABLISHED) {
#endif
while (fState < ESTABLISHED/* && fState != CLOSED*/) {
if (socket->error != B_OK)
return socket->error;
@ -1162,12 +1150,8 @@ TCPEndpoint::_Close()
fFlags |= FLAG_DELETE_ON_CLOSE;
#if 0
// TODO: Should probably be moved here (from _HandleReset()), but cf. the
// comment in _WaitForEstablished().
fSendList.Signal();
_NotifyReader();
#endif
}
@ -1177,12 +1161,6 @@ TCPEndpoint::_HandleReset(status_t error)
socket->error = error;
_Close();
#if 1
// TODO: Should be moved to _Close(), but cf. comment in _WaitForEstablished().
fSendList.Signal();
_NotifyReader();
#endif
gSocketModule->notify(socket, B_SELECT_WRITE, error);
gSocketModule->notify(socket, B_SELECT_ERROR, error);
}
@ -2041,6 +2019,9 @@ TCPEndpoint::_PrepareSendPath(const sockaddr* peer)
fRoute = gDatalinkModule->get_route(Domain(), peer);
if (fRoute == NULL)
return ENETUNREACH;
if ((fRoute->flags & RTF_LOCAL) != 0)
fFlags |= FLAG_LOCAL;
}
// make sure connection does not already exist

2
src/add-ons/kernel/network/protocols/tcp/TCPEndpoint.h
View File

@ -73,6 +73,7 @@ public:
tcp_state State() const { return fState; }
bool IsBound() const;
bool IsLocal() const;
status_t DelayedAcknowledge();
status_t SendAcknowledge(bool force);
@ -162,6 +163,7 @@ private:
net_route *fRoute;
// TODO: don't use a net_route, but a net_route_info!!!
// (the latter will automatically adapt to routing changes)
tcp_sequence fReceiveNext;
tcp_sequence fReceiveMaxAdvertised;

4
src/add-ons/kernel/network/protocols/tcp/tcp.h
View File

@ -25,7 +25,7 @@
class EndpointManager;
typedef enum {
enum tcp_state {
// establishing a connection
CLOSED,
LISTEN,
@ -42,7 +42,7 @@ typedef enum {
FINISH_ACKNOWLEDGED,
CLOSING,
TIME_WAIT
} tcp_state;
};
struct tcp_header {
uint16 source_port;