[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-server-cvs
Subject: [Server-cvs] engine/session clientsession.cpp,1.136,1.137
From: dcollins () helixcommunity ! org
Date: 2009-02-25 21:16:53
Message-ID: 200902252121.n1PLL6tu030356 () mailer ! progressive-comp ! com
[Download RAW message or body]
Update of /cvsroot/server/engine/session
In directory cvs01.internal.helixcommunity.org:/tmp/cvs-serv15737/server/engine/session
Modified Files:
clientsession.cpp
Log Message:
Synopsis
========
Fix for PR 235921: UPTIME: SSPL-triggered memory leak of ClientSession-related \
objects
Branches: SERVER_12_1, HEAD
Reviewer: Jamie
Description
===========
Problem:
~~~~~~~~
The uptime automation switching scripts use the same range of GUIDs
(1-200) for both FCS and SSPL requests. Occasionally there would be
an overlap between an SSPL switch request and an actual connected FCS
client using the same GUID.
Both SSPL and FCS were using the same proc->pc->session_map object.
The GUIDs they are using for the map key are however are not related.
As a result, entries were being made to the map for FCS and removed by
SSPL since they just randomly happened to use the same GUID. This was
resulting in a leak of ClientSession objects, and other related objects.
Solution:
~~~~~~~~~
The solution (as you have probably already guessed :) is to have separate
map instances for FCS and SSPL. They continue to use the same map
implementation, CHXThreadSafeMap.
Note: there still exists a much less frequent leak of these objects,
caused by an unrelated CHXThreadSafeMap-related bug. (It happened three
times in 20 hours, as opposed to thousands of times before the above fix).
I will enter a separate PR for this. One of the problems is a lack of
proper error checking. Things are just added and removed from the maps
without checking whether something is already there or if it failed.
Files Affected
==============
server-restricted/protocol/http-ctrl/fcsgethandler.cpp
server-restricted/protocol/http-ctrl/ssplgethandler.cpp
server/engine/core/core_proc.cpp
server/engine/core/proc_container.cpp
server/engine/core/pub/proc_container.h
server/engine/session/clientsession.cpp
server/protocol/rtsp/crtspbase.cpp
Testing Performed
=================
Unit Tests:
- None
Integration Tests:
- Ran an overnight uptime with instrumentation. This showed that the
above-mentioned leak was fixed.
Leak Tests:
- Printfs demonstrated that the objects that were leaking before were
no longer leaking.
Performance Tests:
- None
Platforms Tested: sunos-5.10-sparc-server
Build verified: sunos-5.10-sparc-server, win2k3-i386-vc7
QA Hints
========
- Regress the PR with new uptimes.
Index: clientsession.cpp
===================================================================
RCS file: /cvsroot/server/engine/session/clientsession.cpp,v
retrieving revision 1.136
retrieving revision 1.137
diff -u -d -r1.136 -r1.137
--- clientsession.cpp 20 Feb 2009 01:01:11 -0000 1.136
+++ clientsession.cpp 25 Feb 2009 21:16:51 -0000 1.137
@@ -393,8 +393,18 @@
if (!m_sGuid.IsEmpty())
{
IUnknown* pSession=0;
- m_pProc->pc->session_map->remove(m_sGuid, pSession);
+
+ if (m_bIsFCSURL)
+ {
+ m_pProc->pc->fcs_session_map->remove(m_sGuid, pSession);
+ }
+ else
+ {
+ m_pProc->pc->sspl_session_map->remove(m_sGuid, pSession);
+ }
+
HX_RELEASE(pSession);
+ m_sGuid.Empty();
}
}
@@ -3153,41 +3163,42 @@
BOOL bValidGuid = FALSE;
IUnknown* pEntry = NULL;
- if (m_bIsFCSURL)
- {
+ if (m_bIsFCSURL)
+ {
bValidGuid = fcsRequestCheck.RetrieveQueryParams(pURL, NULL, \
&m_sGuid, NULL, NULL, &bAllowMDP);
- if(bValidGuid)
+ if (bValidGuid)
{
pEntry = (IUnknown*)(IHXSwitchHandler*)this;
+ m_pProc->pc->fcs_session_map->enter(m_sGuid, pEntry);
}
}
else
{
bValidGuid = ssplRequestCheck.RetrieveQueryParams(pURL, &m_sGuid, \
NULL, NULL, NULL, NULL, NULL);
- if(bValidGuid)
+ if (bValidGuid)
{
pEntry = (IUnknown*)(IHXSSPLSeekHandler*)(m_pPlaylistControl);
+ m_pProc->pc->sspl_session_map->enter(m_sGuid, pEntry);
}
}
- if(bValidGuid)
- {
- m_pProc->pc->session_map->enter(m_sGuid,pEntry);
- if (m_pStats3)
- {
- IHXBuffer* pGUID = NULL;
- if (SUCCEEDED(m_pCommonClassFactory->CreateInstance(IID_IHXBuffer,
- (void**)&pGUID)))
+ if (bValidGuid)
{
- pGUID->Set((const UCHAR*)(const char*)m_sGuid,
- m_sGuid.GetLength() + 1);
- //Used for both FCS (SessionControlID) and SSPL (SessionIDName) \
Logging
- m_pStats3->SetSessionControlId(pGUID);
- HX_RELEASE(pGUID);
+ if (m_pStats3)
+ {
+ IHXBuffer* pGUID = NULL;
+ if \
(SUCCEEDED(m_pCommonClassFactory->CreateInstance(IID_IHXBuffer, + \
(void**)&pGUID))) + {
+ pGUID->Set((const UCHAR*)(const char*)m_sGuid,
+ m_sGuid.GetLength() + 1);
+ //Used for both FCS (SessionControlID) and SSPL \
(SessionIDName) Logging + \
m_pStats3->SetSessionControlId(pGUID); + HX_RELEASE(pGUID);
+ }
+ }
}
}
- }
- }
#ifdef HELIX_FEATURE_SERVER_SSPL
if(m_ulClipBegin)
@@ -5152,8 +5163,18 @@
if (!m_sGuid.IsEmpty())
{
IUnknown* pSession=0;
- m_pProc->pc->session_map->remove(m_sGuid, pSession);
+
+ if (m_bIsFCSURL)
+ {
+ m_pProc->pc->fcs_session_map->remove(m_sGuid, pSession);
+ }
+ else
+ {
+ m_pProc->pc->sspl_session_map->remove(m_sGuid, pSession);
+ }
+
HX_RELEASE(pSession);
+ m_sGuid.Empty();
}
}
_______________________________________________
Server-cvs mailing list
Server-cvs@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/server-cvs
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic