corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 936402: Duplicate connection status in UnixSocketImpl, r=bent

Browse Source 
UnixSocketImpl, which mostly runs on the I/O thread, doesn't control
its reference to UnixSocketConsumer. If the connection status is
stored in UnixSocketConsumer, the I/O thread can't read it safely.

This patch duplicates the connection status in UnixSocketImpl, where
reading from the I/O thread is safe. Methods of UnixSocketImpl don't
need to access mConsumer any longer to obtain the connection status.
This commit is contained in:
Thomas Zimmermann
2013-12-09 14:21:18 +01:00
parent bdbce2b29d
commit 80ad75a960
2 changed files with 29 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
ipc/unixsocket/UnixSocket.cpp
View File

@@ -20,7 +20,6 @@
#include "mozilla/Util.h"
#include "mozilla/FileUtils.h"
#include "nsString.h"
#include "nsThreadUtils.h"
#include "nsTArray.h"
#include "nsXULAppAPI.h"
@@ -44,13 +43,15 @@ class UnixSocketImpl : public MessageLoopForIO::Watcher
{
public:
UnixSocketImpl(UnixSocketConsumer* aConsumer, UnixSocketConnector* aConnector,
const nsACString& aAddress)
const nsACString& aAddress,
SocketConnectionStatus aConnectionStatus)
: mConsumer(aConsumer)
, mIOLoop(nullptr)
, mConnector(aConnector)
, mShuttingDownOnIOThread(false)
, mAddress(aAddress)
, mDelayedConnectTask(nullptr)
, mConnectionStatus(aConnectionStatus)
{
}
@@ -246,6 +247,12 @@ private:
* Task member for delayed connect task. Should only be access on main thread.
*/
CancelableTask* mDelayedConnectTask;
/**
* Socket connection status. Duplicate from UnixSocketConsumer. Should only
* be accessed on I/O thread.
*/
SocketConnectionStatus mConnectionStatus;
};
template<class T>
@@ -526,6 +533,7 @@ UnixSocketImpl::Accept()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -563,6 +571,7 @@ UnixSocketImpl::Connect()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -579,6 +588,7 @@ UnixSocketImpl::Connect()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
if (-1 == fcntl(mFd.get(), F_SETFL, current_opts & ~O_NONBLOCK)) {
@@ -587,6 +597,7 @@ UnixSocketImpl::Connect()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -610,6 +621,7 @@ UnixSocketImpl::Connect()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -625,6 +637,7 @@ UnixSocketImpl::Connect()
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_CONNECTED;
SetUpIO();
}
@@ -721,8 +734,7 @@ UnixSocketImpl::OnFileCanReadWithoutBlocking(int aFd)
MOZ_ASSERT(!NS_IsMainThread());
MOZ_ASSERT(!mShuttingDownOnIOThread);
SocketConnectionStatus status = mConsumer->GetConnectionStatus();
if (status == SOCKET_CONNECTED) {
if (mConnectionStatus == SOCKET_CONNECTED) {
// Read all of the incoming data.
while (true) {
nsAutoPtr<UnixSocketRawData> incoming(new UnixSocketRawData(MAX_READ_SIZE));
@@ -765,9 +777,7 @@ UnixSocketImpl::OnFileCanReadWithoutBlocking(int aFd)
}
MOZ_CRASH("We returned early");
}
if (status == SOCKET_LISTENING) {
} else if (mConnectionStatus == SOCKET_LISTENING) {
int client_fd = accept(mFd.get(), (struct sockaddr*)&mAddr, &mAddrSize);
if (client_fd < 0) {
@@ -792,6 +802,7 @@ UnixSocketImpl::OnFileCanReadWithoutBlocking(int aFd)
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_CONNECTED;
SetUpIO();
}
@@ -804,8 +815,7 @@ UnixSocketImpl::OnFileCanWriteWithoutBlocking(int aFd)
MOZ_ASSERT(!mShuttingDownOnIOThread);
MOZ_ASSERT(aFd >= 0);
SocketConnectionStatus status = mConsumer->GetConnectionStatus();
if (status == SOCKET_CONNECTED) {
if (mConnectionStatus == SOCKET_CONNECTED) {
// Try to write the bytes of mCurrentRilRawData. If all were written, continue.
//
// Otherwise, save the byte position of the next byte to write
@@ -846,7 +856,7 @@ UnixSocketImpl::OnFileCanWriteWithoutBlocking(int aFd)
mOutgoingQ.RemoveElementAt(0);
delete data;
}
} else if (status == SOCKET_CONNECTING) {
} else if (mConnectionStatus == SOCKET_CONNECTING) {
int error, ret;
socklen_t len = sizeof(error);
ret = getsockopt(mFd.get(), SOL_SOCKET, SO_ERROR, &error, &len);
@@ -857,6 +867,7 @@ UnixSocketImpl::OnFileCanWriteWithoutBlocking(int aFd)
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -865,6 +876,7 @@ UnixSocketImpl::OnFileCanWriteWithoutBlocking(int aFd)
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
@@ -874,12 +886,14 @@ UnixSocketImpl::OnFileCanWriteWithoutBlocking(int aFd)
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_ERROR);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_DISCONNECTED;
return;
}
nsRefPtr<OnSocketEventTask> t =
new OnSocketEventTask(this, OnSocketEventTask::CONNECT_SUCCESS);
NS_DispatchToMainThread(t);
mConnectionStatus = SOCKET_CONNECTED;
SetUpIO();
}
@@ -936,7 +950,7 @@ UnixSocketConsumer::ConnectSocket(UnixSocketConnector* aConnector,
}
nsCString addr(aAddress);
mImpl = new UnixSocketImpl(this, connector.forget(), addr);
mImpl = new UnixSocketImpl(this, connector.forget(), addr, SOCKET_CONNECTING);
MessageLoop* ioLoop = XRE_GetIOMessageLoop();
mConnectionStatus = SOCKET_CONNECTING;
if (aDelayMs > 0) {
@@ -962,7 +976,8 @@ UnixSocketConsumer::ListenSocket(UnixSocketConnector* aConnector)
return false;
}
mImpl = new UnixSocketImpl(this, connector.forget(), EmptyCString());
mImpl = new UnixSocketImpl(this, connector.forget(), EmptyCString(),
SOCKET_LISTENING);
mConnectionStatus = SOCKET_LISTENING;
XRE_GetIOMessageLoop()->PostTask(FROM_HERE,
new SocketAcceptTask(mImpl));