Skip to content

Commit

Permalink
Bug 1239979: Cleanup |BluetoothSocket|'s internals when connections c…
Browse files Browse the repository at this point in the history
…lose, r=btian

With this patch, |BluetoothSocket| cleans up its internal state
whenever a connection gets closed, either intentionally or from
an error. The socket can then be reused for a new connection.

If we try to destruct an open Bluetooth socket, we'd probably
leak the file descriptor or transition into an undefined state.
The destructor now asserts that the socket is closed.
  • Loading branch information
tdz committed Jan 21, 2016
1 parent 1331aec commit d1c3bc0
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 28 deletions.
81 changes: 53 additions & 28 deletions dom/bluetooth/bluedroid/BluetoothSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,16 +564,22 @@ BluetoothSocket::BluetoothSocket(BluetoothSocketObserver* aObserver)
, mCurrentRes(nullptr)
, mImpl(nullptr)
{
MOZ_ASSERT(aObserver);

MOZ_COUNT_CTOR_INHERITED(BluetoothSocket, DataSocket);
}

BluetoothSocket::~BluetoothSocket()
{
MOZ_ASSERT(!mImpl); // Socket is closed

MOZ_COUNT_DTOR_INHERITED(BluetoothSocket, DataSocket);
}

void
BluetoothSocket::SetObserver(BluetoothSocketObserver* aObserver)
{
mObserver = aObserver;
}

class ConnectSocketResultHandler final : public BluetoothSocketResultHandler
{
public:
Expand Down Expand Up @@ -755,9 +761,9 @@ BluetoothSocket::Accept(int aListenFd, BluetoothSocketResultHandler* aRes)
void
BluetoothSocket::ReceiveSocketData(nsAutoPtr<UnixSocketBuffer>& aBuffer)
{
MOZ_ASSERT(mObserver);

mObserver->ReceiveSocketData(this, aBuffer);
if (mObserver) {
mObserver->ReceiveSocketData(this, aBuffer);
}
}

// |DataSocket|
Expand All @@ -783,47 +789,41 @@ BluetoothSocket::Close()
return;
}

MOZ_ASSERT(mSocketInterface);
MOZ_ASSERT(mImpl->IsConsumerThread());

// Stop any watching |SocketMessageWatcher|
if (mCurrentRes) {
mSocketInterface->Close(mCurrentRes);
}

// From this point on, we consider mImpl as being deleted.
// We sever the relationship here so any future calls to listen or connect
// will create a new implementation.
mImpl->ShutdownOnConsumerThread();
mImpl->GetIOLoop()->PostTask(FROM_HERE, new SocketIOShutdownTask(mImpl));
mImpl = nullptr;

NotifyDisconnect();
}

void
BluetoothSocket::OnConnectSuccess()
{
MOZ_ASSERT(mObserver);

SetCurrentResultHandler(nullptr);
mObserver->OnSocketConnectSuccess(this);

if (mObserver) {
mObserver->OnSocketConnectSuccess(this);
}
}

void
BluetoothSocket::OnConnectError()
{
MOZ_ASSERT(mObserver);
auto observer = mObserver;

SetCurrentResultHandler(nullptr);
mObserver->OnSocketConnectError(this);
Cleanup();

if (observer) {
observer->OnSocketConnectError(this);
}
}

void
BluetoothSocket::OnDisconnect()
{
MOZ_ASSERT(mObserver);
mObserver->OnSocketDisconnect(this);
auto observer = mObserver;

Cleanup();

if (observer) {
observer->OnSocketDisconnect(this);
}
}

nsresult
Expand All @@ -843,3 +843,28 @@ BluetoothSocket::LoadSocketInterface()

return NS_OK;
}

void
BluetoothSocket::Cleanup()
{
MOZ_ASSERT(mSocketInterface);
MOZ_ASSERT(mImpl);
MOZ_ASSERT(mImpl->IsConsumerThread());

// Stop any watching |SocketMessageWatcher|
if (mCurrentRes) {
mSocketInterface->Close(mCurrentRes);
}

// From this point on, we consider mImpl as being deleted. We
// sever the relationship here so any future calls to listen
// or connect will create a new implementation.
mImpl->ShutdownOnConsumerThread();
mImpl->GetIOLoop()->PostTask(FROM_HERE, new SocketIOShutdownTask(mImpl));
mImpl = nullptr;

mSocketInterface = nullptr;
mObserver = nullptr;
mCurrentRes = nullptr;
mDeviceAddress.Clear();
}
3 changes: 3 additions & 0 deletions dom/bluetooth/bluedroid/BluetoothSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class BluetoothSocket final : public mozilla::ipc::DataSocket
BluetoothSocket(BluetoothSocketObserver* aObserver);
~BluetoothSocket();

void SetObserver(BluetoothSocketObserver* aObserver);

nsresult Connect(const BluetoothAddress& aDeviceAddress,
const BluetoothUuid& aServiceUuid,
BluetoothSocketType aType,
Expand Down Expand Up @@ -89,6 +91,7 @@ class BluetoothSocket final : public mozilla::ipc::DataSocket

private:
nsresult LoadSocketInterface();
void Cleanup();

inline void SetCurrentResultHandler(BluetoothSocketResultHandler* aRes)
{
Expand Down

0 comments on commit d1c3bc0

Please sign in to comment.