Skip to content

Commit 4fbc443

Browse files
bakulfbakulf
committed
Bug 1111971 - A better life-time management of aListener and aContext in WebSocketChannel. r=smaug
CLOSED TREE
1 parent b606c36 commit 4fbc443

File tree

5 files changed

+139
-83
lines changed

5 files changed

+139
-83
lines changed

‎dom/base/WebSocket.cpp

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -646,13 +646,14 @@ WebSocketImpl::DoOnMessageAvailable(const nsACString& aMsg, bool isBinary)
646646
if (NS_FAILED(rv)) {
647647
NS_WARNING("Failed to dispatch the message event");
648648
}
649-
} else {
650-
// CLOSING should be the only other state where it's possible to get msgs
651-
// from channel: Spec says to drop them.
652-
MOZ_ASSERT(readyState == WebSocket::CLOSING,
653-
"Received message while CONNECTING or CLOSED");
649+
650+
return NS_OK;
654651
}
655652

653+
// CLOSING should be the only other state where it's possible to get msgs
654+
// from channel: Spec says to drop them.
655+
MOZ_ASSERT(readyState == WebSocket::CLOSING,
656+
"Received message while CONNECTING or CLOSED");
656657
return NS_OK;
657658
}
658659

@@ -718,14 +719,17 @@ WebSocketImpl::OnStart(nsISupports* aContext)
718719

719720
mWebSocket->SetReadyState(WebSocket::OPEN);
720721

722+
// Let's keep the object alive because the webSocket can be CCed in the
723+
// onopen callback.
724+
nsRefPtr<WebSocket> webSocket = mWebSocket;
725+
721726
// Call 'onopen'
722-
rv = mWebSocket->CreateAndDispatchSimpleEvent(NS_LITERAL_STRING("open"));
727+
rv = webSocket->CreateAndDispatchSimpleEvent(NS_LITERAL_STRING("open"));
723728
if (NS_FAILED(rv)) {
724729
NS_WARNING("Failed to dispatch the open event");
725730
}
726731

727-
mWebSocket->UpdateMustKeepAlive();
728-
732+
webSocket->UpdateMustKeepAlive();
729733
return NS_OK;
730734
}
731735

@@ -1596,23 +1600,27 @@ WebSocketImpl::DispatchConnectionCloseEvents()
15961600

15971601
mWebSocket->SetReadyState(WebSocket::CLOSED);
15981602

1603+
// Let's keep the object alive because the webSocket can be CCed in the
1604+
// onerror or in the onclose callback.
1605+
nsRefPtr<WebSocket> webSocket = mWebSocket;
1606+
15991607
// Call 'onerror' if needed
16001608
if (mFailed) {
16011609
nsresult rv =
1602-
mWebSocket->CreateAndDispatchSimpleEvent(NS_LITERAL_STRING("error"));
1610+
webSocket->CreateAndDispatchSimpleEvent(NS_LITERAL_STRING("error"));
16031611
if (NS_FAILED(rv)) {
16041612
NS_WARNING("Failed to dispatch the error event");
16051613
}
16061614
}
16071615

1608-
nsresult rv = mWebSocket->CreateAndDispatchCloseEvent(mCloseEventWasClean,
1609-
mCloseEventCode,
1610-
mCloseEventReason);
1616+
nsresult rv = webSocket->CreateAndDispatchCloseEvent(mCloseEventWasClean,
1617+
mCloseEventCode,
1618+
mCloseEventReason);
16111619
if (NS_FAILED(rv)) {
16121620
NS_WARNING("Failed to dispatch the close event");
16131621
}
16141622

1615-
mWebSocket->UpdateMustKeepAlive();
1623+
webSocket->UpdateMustKeepAlive();
16161624
Disconnect();
16171625
}
16181626

‎netwerk/protocol/websocket/BaseWebSocketChannel.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "nsILoadGroup.h"
1111
#include "nsIInterfaceRequestor.h"
1212
#include "nsAutoPtr.h"
13+
#include "nsProxyRelease.h"
1314
#include "nsStandardURL.h"
1415

1516
#if defined(PR_LOGGING)
@@ -284,5 +285,26 @@ BaseWebSocketChannel::RetargetDeliveryTo(nsIEventTarget* aTargetThread)
284285
return NS_OK;
285286
}
286287

288+
BaseWebSocketChannel::ListenerAndContextContainer::ListenerAndContextContainer(
289+
nsIWebSocketListener* aListener,
290+
nsISupports* aContext)
291+
: mListener(aListener)
292+
, mContext(aContext)
293+
{
294+
MOZ_ASSERT(NS_IsMainThread());
295+
MOZ_ASSERT(mListener);
296+
}
297+
298+
BaseWebSocketChannel::ListenerAndContextContainer::~ListenerAndContextContainer()
299+
{
300+
MOZ_ASSERT(mListener);
301+
302+
nsCOMPtr<nsIThread> mainThread;
303+
NS_GetMainThread(getter_AddRefs(mainThread));
304+
305+
NS_ProxyRelease(mainThread, mListener, false);
306+
NS_ProxyRelease(mainThread, mContext, false);
307+
}
308+
287309
} // namespace net
288310
} // namespace mozilla

‎netwerk/protocol/websocket/BaseWebSocketChannel.h

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,24 @@ class BaseWebSocketChannel : public nsIWebSocketChannel,
5757
virtual void GetEffectiveURL(nsAString& aEffectiveURL) const = 0;
5858
virtual bool IsEncrypted() const = 0;
5959

60+
class ListenerAndContextContainer MOZ_FINAL
61+
{
62+
public:
63+
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ListenerAndContextContainer)
64+
65+
ListenerAndContextContainer(nsIWebSocketListener* aListener,
66+
nsISupports* aContext);
67+
68+
~ListenerAndContextContainer();
69+
70+
nsCOMPtr<nsIWebSocketListener> mListener;
71+
nsCOMPtr<nsISupports> mContext;
72+
};
73+
6074
protected:
6175
nsCOMPtr<nsIURI> mOriginalURI;
6276
nsCOMPtr<nsIURI> mURI;
63-
nsCOMPtr<nsIWebSocketListener> mListener;
64-
nsCOMPtr<nsISupports> mContext;
77+
nsRefPtr<ListenerAndContextContainer> mListenerMT;
6578
nsCOMPtr<nsIInterfaceRequestor> mCallbacks;
6679
nsCOMPtr<nsILoadGroup> mLoadGroup;
6780
nsCOMPtr<nsILoadInfo> mLoadInfo;

‎netwerk/protocol/websocket/WebSocketChannel.cpp

Lines changed: 65 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -555,30 +555,38 @@ class CallOnMessageAvailable MOZ_FINAL : public nsIRunnable
555555
public:
556556
NS_DECL_THREADSAFE_ISUPPORTS
557557

558-
CallOnMessageAvailable(WebSocketChannel *aChannel,
559-
nsCString &aData,
560-
int32_t aLen)
558+
CallOnMessageAvailable(WebSocketChannel* aChannel,
559+
nsACString& aData,
560+
int32_t aLen)
561561
: mChannel(aChannel),
562+
mListenerMT(aChannel->mListenerMT),
562563
mData(aData),
563564
mLen(aLen) {}
564565

565566
NS_IMETHOD Run() MOZ_OVERRIDE
566567
{
567568
MOZ_ASSERT(mChannel->IsOnTargetThread());
568569

569-
if (mLen < 0)
570-
mChannel->mListener->OnMessageAvailable(mChannel->mContext, mData);
571-
else
572-
mChannel->mListener->OnBinaryMessageAvailable(mChannel->mContext, mData);
570+
if (mListenerMT) {
571+
if (mLen < 0) {
572+
mListenerMT->mListener->OnMessageAvailable(mListenerMT->mContext,
573+
mData);
574+
} else {
575+
mListenerMT->mListener->OnBinaryMessageAvailable(mListenerMT->mContext,
576+
mData);
577+
}
578+
}
579+
573580
return NS_OK;
574581
}
575582

576583
private:
577584
~CallOnMessageAvailable() {}
578585

579-
nsRefPtr<WebSocketChannel> mChannel;
580-
nsCString mData;
581-
int32_t mLen;
586+
nsRefPtr<WebSocketChannel> mChannel;
587+
nsRefPtr<BaseWebSocketChannel::ListenerAndContextContainer> mListenerMT;
588+
nsCString mData;
589+
int32_t mLen;
582590
};
583591
NS_IMPL_ISUPPORTS(CallOnMessageAvailable, nsIRunnable)
584592

@@ -591,30 +599,33 @@ class CallOnStop MOZ_FINAL : public nsIRunnable
591599
public:
592600
NS_DECL_THREADSAFE_ISUPPORTS
593601

594-
CallOnStop(WebSocketChannel *aChannel,
595-
nsresult aReason)
602+
CallOnStop(WebSocketChannel* aChannel,
603+
nsresult aReason)
596604
: mChannel(aChannel),
597-
mReason(aReason) {}
605+
mListenerMT(mChannel->mListenerMT),
606+
mReason(aReason)
607+
{}
598608

599609
NS_IMETHOD Run() MOZ_OVERRIDE
600610
{
601611
MOZ_ASSERT(mChannel->IsOnTargetThread());
602612

603613
nsWSAdmissionManager::OnStopSession(mChannel, mReason);
604614

605-
if (mChannel->mListener) {
606-
mChannel->mListener->OnStop(mChannel->mContext, mReason);
607-
mChannel->mListener = nullptr;
608-
mChannel->mContext = nullptr;
615+
if (mListenerMT) {
616+
mListenerMT->mListener->OnStop(mListenerMT->mContext, mReason);
617+
mChannel->mListenerMT = nullptr;
609618
}
619+
610620
return NS_OK;
611621
}
612622

613623
private:
614624
~CallOnStop() {}
615625

616-
nsRefPtr<WebSocketChannel> mChannel;
617-
nsresult mReason;
626+
nsRefPtr<WebSocketChannel> mChannel;
627+
nsRefPtr<BaseWebSocketChannel::ListenerAndContextContainer> mListenerMT;
628+
nsresult mReason;
618629
};
619630
NS_IMPL_ISUPPORTS(CallOnStop, nsIRunnable)
620631

@@ -627,27 +638,32 @@ class CallOnServerClose MOZ_FINAL : public nsIRunnable
627638
public:
628639
NS_DECL_THREADSAFE_ISUPPORTS
629640

630-
CallOnServerClose(WebSocketChannel *aChannel,
631-
uint16_t aCode,
632-
nsCString &aReason)
641+
CallOnServerClose(WebSocketChannel* aChannel,
642+
uint16_t aCode,
643+
nsACString& aReason)
633644
: mChannel(aChannel),
645+
mListenerMT(mChannel->mListenerMT),
634646
mCode(aCode),
635647
mReason(aReason) {}
636648

637649
NS_IMETHOD Run() MOZ_OVERRIDE
638650
{
639651
MOZ_ASSERT(mChannel->IsOnTargetThread());
640652

641-
mChannel->mListener->OnServerClose(mChannel->mContext, mCode, mReason);
653+
if (mListenerMT) {
654+
mListenerMT->mListener->OnServerClose(mListenerMT->mContext, mCode,
655+
mReason);
656+
}
642657
return NS_OK;
643658
}
644659

645660
private:
646661
~CallOnServerClose() {}
647662

648-
nsRefPtr<WebSocketChannel> mChannel;
649-
uint16_t mCode;
650-
nsCString mReason;
663+
nsRefPtr<WebSocketChannel> mChannel;
664+
nsRefPtr<BaseWebSocketChannel::ListenerAndContextContainer> mListenerMT;
665+
uint16_t mCode;
666+
nsCString mReason;
651667
};
652668
NS_IMPL_ISUPPORTS(CallOnServerClose, nsIRunnable)
653669

@@ -658,25 +674,29 @@ NS_IMPL_ISUPPORTS(CallOnServerClose, nsIRunnable)
658674
class CallAcknowledge MOZ_FINAL : public nsCancelableRunnable
659675
{
660676
public:
661-
CallAcknowledge(WebSocketChannel *aChannel,
662-
uint32_t aSize)
677+
CallAcknowledge(WebSocketChannel* aChannel,
678+
uint32_t aSize)
663679
: mChannel(aChannel),
680+
mListenerMT(mChannel->mListenerMT),
664681
mSize(aSize) {}
665682

666683
NS_IMETHOD Run()
667684
{
668685
MOZ_ASSERT(mChannel->IsOnTargetThread());
669686

670687
LOG(("WebSocketChannel::CallAcknowledge: Size %u\n", mSize));
671-
mChannel->mListener->OnAcknowledge(mChannel->mContext, mSize);
688+
if (mListenerMT) {
689+
mListenerMT->mListener->OnAcknowledge(mListenerMT->mContext, mSize);
690+
}
672691
return NS_OK;
673692
}
674693

675694
private:
676695
~CallAcknowledge() {}
677696

678-
nsRefPtr<WebSocketChannel> mChannel;
679-
uint32_t mSize;
697+
nsRefPtr<WebSocketChannel> mChannel;
698+
nsRefPtr<BaseWebSocketChannel::ListenerAndContextContainer> mListenerMT;
699+
uint32_t mSize;
680700
};
681701

682702
//-----------------------------------------------------------------------------
@@ -1174,17 +1194,7 @@ WebSocketChannel::~WebSocketChannel()
11741194
NS_ProxyRelease(mainThread, forgettable, false);
11751195
}
11761196

1177-
if (mListener) {
1178-
nsIWebSocketListener *forgettableListener;
1179-
mListener.forget(&forgettableListener);
1180-
NS_ProxyRelease(mainThread, forgettableListener, false);
1181-
}
1182-
1183-
if (mContext) {
1184-
nsISupports *forgettableContext;
1185-
mContext.forget(&forgettableContext);
1186-
NS_ProxyRelease(mainThread, forgettableContext, false);
1187-
}
1197+
mListenerMT = nullptr;
11881198

11891199
if (mLoadGroup) {
11901200
nsILoadGroup *forgettableGroup;
@@ -1586,7 +1596,7 @@ WebSocketChannel::ProcessInput(uint8_t *buffer, uint32_t count)
15861596
LOG(("WebSocketChannel:: %stext frame received\n",
15871597
isDeflated ? "deflated " : ""));
15881598

1589-
if (mListener) {
1599+
if (mListenerMT) {
15901600
nsCString utf8Data;
15911601

15921602
if (isDeflated) {
@@ -1657,7 +1667,7 @@ WebSocketChannel::ProcessInput(uint8_t *buffer, uint32_t count)
16571667
mCloseTimer->Cancel();
16581668
mCloseTimer = nullptr;
16591669
}
1660-
if (mListener) {
1670+
if (mListenerMT) {
16611671
mTargetThread->Dispatch(new CallOnServerClose(this, mServerCloseCode,
16621672
mServerCloseReason),
16631673
NS_DISPATCH_NORMAL);
@@ -1696,7 +1706,7 @@ WebSocketChannel::ProcessInput(uint8_t *buffer, uint32_t count)
16961706
LOG(("WebSocketChannel:: %sbinary frame received\n",
16971707
isDeflated ? "deflated " : ""));
16981708

1699-
if (mListener) {
1709+
if (mListenerMT) {
17001710
nsCString binaryData;
17011711

17021712
if (isDeflated) {
@@ -2265,8 +2275,11 @@ WebSocketChannel::StopSession(nsresult reason)
22652275

22662276
if (!mCalledOnStop) {
22672277
mCalledOnStop = 1;
2268-
mTargetThread->Dispatch(new CallOnStop(this, reason),
2269-
NS_DISPATCH_NORMAL);
2278+
2279+
nsWSAdmissionManager::OnStopSession(this, reason);
2280+
2281+
nsRefPtr<CallOnStop> runnable = new CallOnStop(this, reason);
2282+
mTargetThread->Dispatch(runnable, NS_DISPATCH_NORMAL);
22702283
}
22712284
}
22722285

@@ -2577,10 +2590,10 @@ WebSocketChannel::StartWebsocketData()
25772590
nsWSAdmissionManager::OnConnected(this);
25782591

25792592
LOG(("WebSocketChannel::StartWebsocketData Notifying Listener %p\n",
2580-
mListener.get()));
2593+
mListenerMT ? mListenerMT->mListener.get() : nullptr));
25812594

2582-
if (mListener) {
2583-
mListener->OnStart(mContext);
2595+
if (mListenerMT) {
2596+
mListenerMT->mListener->OnStart(mListenerMT->mContext);
25842597
}
25852598

25862599
// Start keepalive ping timer, if we're using keepalive.
@@ -2943,7 +2956,7 @@ WebSocketChannel::AsyncOpen(nsIURI *aURI,
29432956
return NS_ERROR_UNEXPECTED;
29442957
}
29452958

2946-
if (mListener || mWasOpened)
2959+
if (mListenerMT || mWasOpened)
29472960
return NS_ERROR_ALREADY_OPENED;
29482961

29492962
nsresult rv;
@@ -3109,8 +3122,7 @@ WebSocketChannel::AsyncOpen(nsIURI *aURI,
31093122
// Only set these if the open was successful:
31103123
//
31113124
mWasOpened = 1;
3112-
mListener = aListener;
3113-
mContext = aContext;
3125+
mListenerMT = new ListenerAndContextContainer(aListener, aContext);
31143126
IncrementSessionCount();
31153127

31163128
return rv;

0 commit comments

Comments
 (0)