[Common-dev] Re: [Filesystem-dev] Fwd: CR-Client: bug 93416 fix, HTTP & surestream
Henry Ping ping at real.comlooks good, please check in for HEAD and 1_1_6_neptune
-->Henry
At 10:11 PM 2/27/2004 -0800, Bob Clark wrote:
>There were a couple cases that didn't work with this diff I sent out
>earlier. I've worked those out and frankly I'm a little happier because
>now it's less invasive for the vast majority of cases.
>
>(The minority of cases where behavior has changed is surestream being
>http-streamed as well as the (I'd expect) rare case where a smil or ram
>has the same http-streamed file multiple times. Those will also use the
>same chunky res.)
>
>Henry, I've answered your inline questions here at the top, then I'll
>briefly discuss the new diff.
>
>
>>pCursorOwner needs to be AddRef in AddCursor() and Released in
>>RemoveCursor() to ensure the file object
>>won't be deleted accidently by other components.
>
>pCursorOwner -- from the CChunkyRes's perspective -- is used only as an
>ID, so it's never dereferenced; this removes the "danger", I guess, of
>trying to dereference deleted objects.
>
>The real reason this is troublesome though is that the http file object
>holds onto the m_pChunkyRes object until its (the http file object's)
>destructor. In order to close things down in a way where reference
>counting could be effective would require more fiddling in the code
>than I'm comfortable with at this juncture. (I'm trying to keep
>collateral damage to a minimum.)
>
>>m_CursorLocationMap is not used. Actually I think it's better to use
>>map instead of list for faster lookup since
>>SetCursor() is called very frequently.
>
>Good idea, Henry. The latest diff uses a map.
>
>>I suggest to also define a new method GetCursorInfo() which takes
>>pCursorOwner and returns ulCursorLocation, see reason below.
>
>Thanks, Henry. That's downright sensible too. You'll see that I've
>incorporated this suggestion as well.
>
>
>(when closing the socket in _EnsureThatWeAreReadingWisely()...)
>
>>So we will stop reading from the socket, but we should still keep
>>reading from the ChunkyRes to satisfy
>>the read request from the file format, correct?
>
>That's correct -- we can close the socket because a different http file
>object's socket is writing to the (shared) ChunkyRes. If the other http
>file object stops "being helpful" this one will re-open with a byte
>range request.
>
>>>>- ULONG32 ulLength =
>>>>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition) +
>>>>m_ulCurrentReadPosition;
>>>>- _HandleByteRangeSeek(ulLength);
>>>>+ ULONG32 ulLength =
>>>>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition);
>>>>+ _HandleByteRangeSeek(m_ulCurrentReadPosition +
>>>>ulLength);
>>
>>The above changes don't seems to be related to this multiple
>>downloads' fix?
>
>No, it's just that "ulLength" wasn't strictly being used as a length;
>_HandleByteRangeSeek expects an absolute offset, and it was just
>confusing to me to pass in a variable named "length". The math works
>out the same, it just doesn't irritate me.
>
>>Can you also verify the same HTTP URL defined multiple times in RAM
>>and SMIL(within par) are
>>working properly?
>
>I've done these repeatedly on my Mac as well as on my Windows machine,
>in addition to the original repro.
>
>
>
>I've included a new diff here. In addition to taking Henry's
>suggestions into account, I've been able to remove that bHasPartialData
>logic. I'm pleased with the simpler version because now I believe that
>there is no difference for the (majority) case of a single http file
>object being associated with a single chunky res object.
>
>For the case where several http file objects are sharing a chunky res,
>one of the implications is that this chunky res is essentially being
>filled in "behind its back", that is, there may be assumptions about
>the order the data is read in. One of these is how and when it sets
>m_bReadContentsDone to true; it only does so if (1) it has actually
>read all the contents or (2) its chunky res only has one cursor -- i.e.
>nobody else is writing behind its back. That second case is the most
>obvious example of deliberately trying to keep the majority case
>working as it always has.
>
>--Bob
>
>Index: chunkres.cpp
>===================================================================
>RCS file: /cvsroot/common/fileio/chunkres.cpp,v
>retrieving revision 1.14
>diff -u -w -r1.14 chunkres.cpp
>--- chunkres.cpp 15 Jul 2003 20:14:32 -0000 1.14
>+++ chunkres.cpp 28 Feb 2004 05:48:34 -0000
>@@ -602,6 +602,7 @@
>
> ULONG32 CChunkyRes::GetContiguousLength(ULONG32 offset /* = 0 */)
> {
>+ Lock();
> ULONG32 contiguousLength = 0;
> int ndx;
>
>@@ -643,6 +644,7 @@
> }
>
> exit:
>+ Unlock();
> return contiguousLength;
> }
>
>@@ -670,6 +672,7 @@
> //
> HX_RESULT CChunkyRes::GetData(ULONG32 offset, char* buf, ULONG32
>count, ULONG32* actual)
> {
>+ Lock();
> HX_RESULT theErr = HXR_OK;
> int ndx;
>
>@@ -755,12 +758,14 @@
>
> exit:
>
>+ Unlock();
> return theErr;
> }
>
> HX_RESULT
> CChunkyRes::GetContiguousDataPointer(ULONG32 offset, char*& buf,
>ULONG32 count)
> {
>+ Lock();
> HX_RESULT theErr = HXR_OK;
> HX_ASSERT(m_bDiscardUsedData == FALSE && m_bDisableDiskIO ==
>FALSE);
>
>@@ -802,6 +807,7 @@
> ULONG32 chunkAmount = min(DEF_CHUNKYRES_CHUNK_SIZE-chunkOffset,
>count);
>
> theErr =
>pChunk->GetContiguousDataPointer(chunkOffset,buf,chunkAmount);
>+ Unlock();
> return theErr;
> }
>
>@@ -826,8 +832,11 @@
> // HX_RESULT
> // Possible errors include: TBD.
> //
>-HX_RESULT CChunkyRes::SetData(ULONG32 offset, const char* buf, ULONG32
>count)
>+
>+HX_RESULT CChunkyRes::SetData(ULONG32 offset, const char* buf, ULONG32
>count, void* pOwner /* = NULL */)
> {
>+ Lock();
>+
> HX_RESULT theErr = HXR_OK;
>
> ULONG32 ulFirstChunk = offset/DEF_CHUNKYRES_CHUNK_SIZE;
>@@ -877,8 +886,15 @@
> chunkOffset = 0;
> }
>
>+ // maintain the cursor, if this SetData includes a cursor
>owner.
>+ if (pOwner)
>+ {
>+ SetCursor(pOwner, offset + count);
>+ }
>+
> exit:
>
>+ Unlock();
> return theErr;
> }
>
>@@ -914,6 +930,81 @@
> }
> }
>
>+void
>+CChunkyRes::AddCursor(void* pCursorOwner, ULONG32 ulCursorLocation)
>+{
>+ HX_ASSERT(m_CursorMap.Lookup(pCursorOwner) == NULL);
>+ m_CursorMap.SetAt(pCursorOwner, (void*)ulCursorLocation);
>+}
>+
>+void
>+CChunkyRes::SetCursor(void* pCursorOwner, ULONG32 ulCursorLocation)
>+{
>+ HX_ASSERT(m_CursorMap.Lookup(pCursorOwner) != NULL);
>+ m_CursorMap.SetAt(pCursorOwner, (void*)ulCursorLocation);
>+}
>+
>+void
>+CChunkyRes::RemoveCursor(void* pCursorOwner)
>+{
>+ HX_ASSERT(m_CursorMap.Lookup(pCursorOwner) != NULL);
>+ m_CursorMap.Remove(pCursorOwner);
>+}
>+
>+ULONG32
>+CChunkyRes::CountCursors()
>+{
>+ return m_CursorMap.GetCount();
>+}
>+
>+HX_RESULT
>+CChunkyRes::GetNthCursorInformation(int nIndex,
>REF(void*)pCursorOwner, REF(ULONG32)ulCursorLocation)
>+{
>+ HX_ASSERT(nIndex >= 0);
>+ HX_ASSERT(nIndex < m_CursorMap.GetCount());
>+
>+ int ndx = 0;
>+
>+ LISTPOSITION pos = m_CursorMap.GetStartPosition();
>+ while (ndx < nIndex)
>+ {
>+ void* pDummyOwner = NULL;
>+ ULONG32 ulDummyLocation = 0;
>+ ndx++;
>+ m_CursorMap.GetNextAssoc(pos, pDummyOwner,
>(void*&)ulDummyLocation);
>+
>+ if (!pos)
>+ {
>+ return HXR_UNEXPECTED;
>+ }
>+ }
>+
>+ HX_ASSERT(pos);
>+
>+ if (!pos)
>+ {
>+ return HXR_UNEXPECTED;
>+ }
>+
>+ m_CursorMap.GetNextAssoc(pos, pCursorOwner,
>(void*&)ulCursorLocation);
>+
>+ return HXR_OK;
>+}
>+
>+HX_RESULT
>+CChunkyRes::GetCursorInformation(void* pInCursorOwner,
>REF(ULONG32)ulOutCursorLocation)
>+{
>+ if (m_CursorMap.Lookup(pInCursorOwner,
>(void*&)ulOutCursorLocation))
>+ {
>+ return HXR_OK;
>+ }
>+ else
>+ {
>+ return HXR_UNEXPECTED;
>+ }
>+}
>+
>+
>
>/////////////////////////////////////////////////////////////////////// //////
> //
> // Method:
>@@ -988,6 +1079,7 @@
>
> HX_ASSERT(m_ChunksMemoryMRU);
> HX_ASSERT(m_ChunksDiskMRU);
>+
> }
>
>
>/////////////////////////////////////////////////////////////////////// //////
>
>
>Index: pub/chunkres.h
>===================================================================
>RCS file: /cvsroot/common/fileio/pub/chunkres.h,v
>retrieving revision 1.3
>diff -u -w -r1.3 chunkres.h
>--- pub/chunkres.h 4 Mar 2003 04:20:02 -0000 1.3
>+++ pub/chunkres.h 28 Feb 2004 05:48:34 -0000
>@@ -215,7 +215,7 @@
> HX_RESULT SetData
> (
> ULONG32
> offset, const char* buf,
>- ULONG32 count
>+ ULONG32
>count, void* pOwner = NULL
> );
> void Lock()
> {
>@@ -240,6 +240,16 @@
>
> CChunkyRes();
> ~CChunkyRes();
>+
>+ void AddCursor(void* pCursorOwner, ULONG32 ulCursorLocation =
>0);
>+ void SetCursor(void* pCursorOwner, ULONG32 ulCursorLocation);
>+ void RemoveCursor(void* pCursorOwner);
>+ ULONG32 CountCursors();
>+ HX_RESULT GetNthCursorInformation(int nIndex,
>REF(void*)pCursorOwner, REF(ULONG32)ulCursorLocation);
>+ HX_RESULT GetCursorInformation(void* pInCursorOwner,
>REF(ULONG32)ulOutCursorLocation);
>+
>+private:
>+ CHXMapPtrToPtr m_CursorMap;
> };
>
> ///////////////////////////////////////////////////////////////
>
>
>Index: httpfsys.cpp
>===================================================================
>RCS file: /cvsroot/filesystem/http/httpfsys.cpp,v
>retrieving revision 1.28
>diff -u -w -r1.28 httpfsys.cpp
>--- httpfsys.cpp 3 Dec 2003 18:38:35 -0000 1.28
>+++ httpfsys.cpp 28 Feb 2004 05:48:50 -0000
>@@ -186,6 +186,9 @@
>
> static INT32 g_nRefCount_http = 0;
>
>+static CChunkyResMap g_ChunkyResMap;
>+
>+
> #define WWW_AUTHENTICATION_RECENT_KEY
>"authentication.http.realm.recent"
> #define PROXY_AUTHENTICATION_RECENT_KEY
>"proxy-authentication.http.realm.recent"
>
>@@ -205,6 +208,69 @@
> }
> #endif
>
>+// ChunkyResMap is a manager-type class that is able to re-use the
>same ChunkyRes for different
>+// file objects that use the same url. This happens, for example, if
>you're http-streaming a
>+// surestream file which may have many logical streams pasted one
>after another.
>+
>+CChunkyResMap::CChunkyResMap()
>+{
>+ m_pChunkyResURLMap = new CHXMapStringToOb();
>+}
>+
>+CChunkyResMap::~CChunkyResMap()
>+{
>+ delete m_pChunkyResURLMap;
>+}
>+
>+CChunkyRes*
>+CChunkyResMap::GetChunkyResForURL(const char* pURL, void* pCursorOwner)
>+{
>+ CChunkyRes* pChunkyRes = NULL;
>+
>+ if ( !m_pChunkyResURLMap->Lookup( pURL, (void*&)pChunkyRes ) )
>+ {
>+ pChunkyRes = new CChunkyRes();
>+ m_pChunkyResURLMap->SetAt(pURL, pChunkyRes);
>+ }
>+
>+ HX_ASSERT(pChunkyRes);
>+ pChunkyRes->AddCursor(pCursorOwner);
>+
>+ return pChunkyRes;
>+}
>+
>+void
>+CChunkyResMap::RelinquishChunkyRes(CChunkyRes* pChunkyRes, void*
>pCursorOwner)
>+{
>+ if (!pChunkyRes) return;
>+
>+ CChunkyRes* pChunkyResInMap = NULL;
>+ CHXString strURL;
>+
>+ LISTPOSITION pos;
>+ LISTPOSITION prevPos = NULL;
>+
>+ pos = m_pChunkyResURLMap->GetStartPosition();
>+
>+
>+ while (pos)
>+ {
>+ prevPos = pos;
>+ m_pChunkyResURLMap->GetNextAssoc(pos, strURL,
>(void*&)pChunkyResInMap);
>+ if (pChunkyRes == pChunkyResInMap)
>+ {
>+ pChunkyRes->RemoveCursor(pCursorOwner);
>+
>+ if (pChunkyRes->CountCursors() == 0)
>+ {
>+ m_pChunkyResURLMap->RemoveKey(strURL);
>+ delete pChunkyResInMap;
>+ }
>+ }
>+ }
>+}
>+
>+
>
>/ ************************************************************************
>****
> *
> * Function:
>@@ -1049,30 +1115,6 @@
> }
> }
>
>- m_pChunkyRes = new CChunkyRes;
>-
>- #ifdef CREATE_LOG_DUMP
>- FILE *fp = fopen(LOG_DUMP_FILE, "a+");
>- if (fp)
>- {
>- fprintf(fp, "NewChunky %lx\n", m_pChunkyRes);
>- fclose(fp);
>- }
>- #endif
>-
>- if (m_bOnServer)
>- {
>- m_pChunkyRes->DisableDiskIO();
>- }
>-
>-#if defined(HELIX_FEATURE_HTTP_GZIP)
>- m_pDecoder = new CDecoder;
>- if (m_pDecoder && m_pChunkyRes)
>- {
>- m_pDecoder->SetOutputSink(m_pChunkyRes);
>- }
>-#endif
>-
> m_pCallback = HTTPFileObjCallback::CreateObject();
> if (m_pCallback)
> {
>@@ -1240,6 +1282,9 @@
> m_pPAC->AbortProxyInfo(this);
> }
>
>+ g_ChunkyResMap.RelinquishChunkyRes(m_pChunkyRes, this);
>+ m_pChunkyRes = NULL;
>+
> HX_RELEASE(m_pFileSystem);
> HX_RELEASE(m_pRequest);
> HX_RELEASE(m_pRequestHeadersOrig);
>@@ -1292,7 +1337,6 @@
> #if defined(HELIX_FEATURE_HTTP_GZIP)
> HX_DELETE(m_pDecoder);
> #endif
>- HX_DELETE(m_pChunkyRes);
>
> if (m_pChunkedEncoding)
> {
>@@ -1577,7 +1621,9 @@
> // m_bByteRangeSeekPending means we reconnecting the socket
> // a new location.
> if (m_bByteRangeSeekPending)
>+ {
> m_bSeekPending = TRUE;
>+ }
> else
> {
> #ifdef CREATE_LOG_DUMP
>@@ -2032,6 +2078,45 @@
>
>
>/ ************************************************************************
> * Method:
>+ * Private interface::InitializeChunkyRes
>+ * Purpose:
>+ * This encapsulates initialization of the chunky res and
>associated
>+ * objects
>+ */
>+HX_RESULT CHTTPFileObject::_InitializeChunkyRes(const char* url)
>+{
>+ if (!m_pChunkyRes)
>+ {
>+ m_pChunkyRes = g_ChunkyResMap.GetChunkyResForURL(url, this);
>+
>+#ifdef CREATE_LOG_DUMP
>+ FILE *fp = fopen(LOG_DUMP_FILE, "a+");
>+ if (fp)
>+ {
>+ fprintf(fp, "NewChunky %lx\n", m_pChunkyRes);
>+ fclose(fp);
>+ }
>+#endif
>+
>+ if (m_bOnServer)
>+ {
>+ m_pChunkyRes->DisableDiskIO();
>+ }
>+
>+#if defined(HELIX_FEATURE_HTTP_GZIP)
>+ m_pDecoder = new CDecoder;
>+ if (m_pDecoder && m_pChunkyRes)
>+ {
>+ m_pDecoder->SetOutputSink(m_pChunkyRes);
>+ }
>+#endif
>+
>+ }
>+ return HXR_OK;
>+}
>+
>+/ ************************************************************************
>+ * Method:
> * Private interface::OpenFile
> * Purpose:
> * This common method is used from Init() and
>GetFileObjectFromPool()
>@@ -2048,6 +2133,8 @@
> IHXBuffer* pProxyPort = NULL;
> IHXProxyManager* pProxyManager = NULL;
>
>+ _InitializeChunkyRes(url);
>+
> LOG("_OpenFile");
> LOGX((szDbgTemp, " URL='%s'", url));
> // Make local copy of url
>@@ -2276,6 +2363,7 @@
> }
> else
> {
>+
> /* connect to the host and start getting the data */
>
> theErr = BeginGet(m_uByteRangeSeekOffset);
>@@ -2648,6 +2736,15 @@
> continue; // CallReadDone(HXR_FAIL..) for remaining
>reads in pending list
> }
> }
>+ else if (m_bKnowContentSize
>+ && ((m_ulCurrentReadPosition + ulReadCount) >=
>m_nContentSize)
>+ &&
>(m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition) >=
>m_nContentSize - m_ulCurrentReadPosition))
>+ {
>+ // OK, so we've read past the end of the file but -- BUT!
>-- we may
>+ // be writing "behind ourselves" or something. The math is
>slightly
>+ // different from the m_bReadContentsDone case.
>+ ulReadCount = m_nContentSize - m_ulCurrentReadPosition;
>+ }
> else
> {
> // We have to wait for more data to come in the from
>socket
>@@ -3488,8 +3585,15 @@
> #endif
>
> m_pChunkyRes->SetData(m_nContentRead,
>- (char*)pBuffer->GetBuffer(), size);
>+ (char*)pBuffer->GetBuffer(), size, this);
> m_nContentRead += size;
>+
>+ // xxxbobclark it's possible -- especially if we have
>several
>+ // http file objects sharing a single chunky res (like
>if we're
>+ // accessing a surestream file) -- that we might
>stumble onto
>+ // parts of the file that we've already downloaded; by
>calling
>+ // _Ensure() here we can avoid redundant downloading.
>+ _EnsureThatWeAreReadingWisely();
> }
>
> if(m_bKnowContentSize && m_nContentRead >= m_nContentSize)
>@@ -4184,9 +4288,56 @@
> // then seeked backwards, and has now finished reading
> // up to the already-valid place. So we need to
> // kick-start reading way down the line.
>+
>+ // xxxbobclark now here's the problem: we can easily
>end up with two separate
>+ // http file objs writing into the same chunky res
>here. If they're writing into
>+ // the same range we'll end up with "leapfrogging"
>cursors, where each iteration
>+ // will do a whole nuther byte range seek to the last
>set byte that the other
>+ // guy just wrote. (This is kind of fun to watch in
>action, but less than
>+ // optimal in the real world.) What I'd rather do
>instead is see if another obj
>+ // is actively writing into this range and if so I'll
>just stop reading.
>+
>+ BOOL bSomebodyElseOwnsThisRange = FALSE;
>+ {
>+ int max = m_pChunkyRes->CountCursors();
>+ int ndx;
>+ for (ndx = 0; ndx < max; ndx++)
>+ {
>+ void* pOwner;
>+ ULONG32 ulCursorLocation;
>+ m_pChunkyRes->GetNthCursorInformation(ndx,
>pOwner, ulCursorLocation);
>+
>+ if (ulCursorLocation ==
>m_ulCurrentReadPosition + ulLength)
>+ {
>+ if (pOwner != this)
>+ {
>+ bSomebodyElseOwnsThisRange = TRUE;
>+ }
>+ }
>+ }
>+ }
>+
>+ if (bSomebodyElseOwnsThisRange)
>+ {
>+ // If there is a pending callback, be sure to
>remove it!
>+ // (there might be the connection timeout callback
>pending)
>+ if (m_pCallback &&
>+ m_pCallback->m_bCallbackPending &&
>+ m_pCallback->m_ulPendingCallbackID &&
>+ m_pScheduler)
>+ {
>+
>m_pScheduler->Remove(m_pCallback->m_ulPendingCallbackID);
>+ m_pCallback->m_bCallbackPending = FALSE;
>+ }
>+
>+ HX_RELEASE(m_pSocket);
>+ }
>+ else
>+ {
> _HandleByteRangeSeek(m_ulCurrentReadPosition + ulLength);
> }
> }
>+ }
> else
> {
> // XXXbobclark OK, it's plausible that something could be
>@@ -4199,9 +4350,13 @@
> // of the file. If not, then kick-start another byte range
> // seek.
>
>- // This is the number of contigous bytes we have
>- ULONG32 ulLength =
>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition) +
>m_ulCurrentReadPosition;
>- _HandleByteRangeSeek(ulLength);
>+ // This is the number of contiguous bytes we have
>+ ULONG32 ulLength =
>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition);
>+ BOOL bAtTheEnd = (m_bKnowContentSize &&
>((m_ulCurrentReadPosition + ulLength) == m_nContentSize));
>+ if (!bAtTheEnd)
>+ {
>+ _HandleByteRangeSeek(m_ulCurrentReadPosition +
>ulLength);
>+ }
>
> //HX_ASSERT(!"Reading BEHIND the current seek position");
> }
>@@ -4871,7 +5026,7 @@
>
> m_pChunkyRes->SetData(m_nContentRead,
> (const char*)pBuffer->GetBuffer() + ulHeaderLength,
>- nContentLen);
>+ nContentLen, this);
> m_nContentRead += nContentLen;
>
> }
>@@ -4887,8 +5042,23 @@
> // read to the end of the content regardless of the seek
> // since m_bReadContentDone will be reset to FALSE when
> // seek occurs
>+
>+ // xxxbobclark if there are several HTTP objects writing
>+ // to a single chunky res (like a surestream file or
>+ // a situation involving ram or smil with multiple
>references
>+ // to the same HTTP-streamed media) it's possible that the
>+ // file may be completed at a "surprising" time -- i.e.
>+ // without this object really knowing about it. So we'll
>+ // only mark it done if we can verify that the whole chunky
>+ // res has been filled, or if there's only a single cursor
>+ // writing to it.
>+
>+ if ((m_pChunkyRes->GetContiguousLength(0) ==
>m_nContentSize)
>+ || (m_pChunkyRes->CountCursors() == 1))
>+ {
> m_bReadContentsDone = TRUE;
> }
>+ }
>
> if(m_bStatPending)
> {
>@@ -5733,7 +5903,7 @@
> #endif
>
> // copy the data from the cache
>- m_pChunkyRes->SetData(0, (const char *)dbtContent.data,
>dbtContent.size);
>+ m_pChunkyRes->SetData(0, (const char *)dbtContent.data,
>dbtContent.size, this);
>
> m_bCached = TRUE;
> // These are values expected to be set when data is
>downloaded
>@@ -7166,7 +7336,7 @@
> }
> #endif
>
>- m_pChunkyRes->SetData(m_nContentRead,
>pChunkedEncoding->buf, pChunkedEncoding->size);
>+ m_pChunkyRes->SetData(m_nContentRead,
>pChunkedEncoding->buf, pChunkedEncoding->size, this);
> m_nContentRead += pChunkedEncoding->size;
>
> memset(pChunkedEncoding->buf, 0, MAX_CHUNK_SIZE);
>
>
>Index: httpfsys.h
>===================================================================
>RCS file: /cvsroot/filesystem/http/httpfsys.h,v
>retrieving revision 1.5
>diff -u -w -r1.5 httpfsys.h
>--- httpfsys.h 8 Oct 2003 16:49:10 -0000 1.5
>+++ httpfsys.h 28 Feb 2004 05:48:50 -0000
>@@ -73,6 +73,7 @@
> class CChunkyRes;
> class CHXSimpleList;
> class CDecoder;
>+class CHXMapStringToOb;
>
> class HTTPTCPResponse;
> class HTTPFileObjCallback;
>@@ -96,6 +97,21 @@
> char* buf;
> } HTTPChunkedEncoding;
>
>+
>+class CChunkyResMap
>+{
>+public:
>+ CChunkyResMap::CChunkyResMap();
>+ virtual CChunkyResMap::~CChunkyResMap();
>+ CChunkyRes* GetChunkyResForURL(const char* pURL, void*
>pCursorOwner);
>+ void RelinquishChunkyRes(CChunkyRes* pChunkyRes, void*
>pCursorOwner);
>+
>+private:
>+ CHXMapStringToOb* m_pChunkyResURLMap;
>+};
>+
>+
>+
>
>/////////////////////////////////////////////////////////////////////// //////
> //
> // Class:
>@@ -389,6 +405,8 @@
>
> HX_RESULT _ReOpen();
>
>+ HX_RESULT _InitializeChunkyRes(const char* url);
>+
>
>/ ************************************************************************
> * Method:
> * Private interface::_OpenFile
>
>
>
>
>end of diff -- I've kept the previous diff and notes for context.
>--Bob
>
>
>
>On Wednesday, February 25, 2004, at 11:32 PM, Henry Ping wrote:
>
>>comments inline
>>
>>At 01:33 PM 2/25/2004 -0800, Henry Ping wrote:
>>>Forwarded to dev at filesystem and dev at common
>>>
>>>-->Henry
>>>
>>>>Delivered-To: ping at real.com
>>>>Date: Wed, 25 Feb 2004 13:14:06 -0800
>>>>Subject: CR-Client: bug 93416 fix, HTTP & surestream
>>>>From: Bob Clark <bobclark at real.com>
>>>>To: neptune at real.com
>>>>X-Mailer: Apple Mail (2.553)
>>>>
>>>>(should I post this CR to helixdev and/or dev at client and/or
>>>>dev at common?)
>>>>
>>>>Modified by:
>>>>
>>>> bobclark at real.com
>>>>
>>>>
>>>>Date:
>>>>
>>>> 2/25/04
>>>>
>>>>Synopsis:
>>>>
>>>> SureStream files being streamed via HTTP/1.1 were being triple- (or
>>>>quadruple-, or quintuple-) downloading the file. This fix ensures
>>>>that
>>>>any given bit of the file is only downloaded once.
>>>>
>>>>
>>>>Overview:
>>>>
>>>> It's admittedly an odd thing to HTTP-stream a SureStream file, but
>>>>when it does happen, downloading most of the file three or more times
>>>>is ... suboptimal. Before this change, httpfsys's file objects each
>>>>owned a single CChunkyRes that held the file being downloaded from
>>>>the
>>>>http server. What this change does is to allow multiple http file
>>>>objects (which are using the same URL) to share a CChunkyRes.
>>>> By sharing a single CChunkyRes object, each http file object can
>>>>now
>>>>detect already-downloaded ranges, even if it was a different http
>>>>file
>>>>object that filled in that range.
>>>> CChunkyRes now has the notion of a list of cursors -- the list of
>>>>who
>>>>is writing into it, and *where* they are writing into it.
>>>> CHTTPFileObject now uses its URL to grab a CChunkyRes from a map;
>>>>thus all simultaneously-existing http file objects with the same URL
>>>>are sharing the same CChunkyRes.
>>>>
>>>> Making invasive changes to httpfsys at this late date is fraught
>>>>with
>>>>danger, so my main goal is "first, do no harm". I've tried to
>>>>minimize
>>>>the *changes* in existing code, and tried to keep the code paths as
>>>>similar as possible for cases where this bug does not appear. I've
>>>>tried to put the new behavior into new code, which only gets called
>>>>if
>>>>this bug would be happening.
>>>> First I'll outline the *changes*, then I'll go into the
>>>>*additions*.
>>>>The changes are what I'm more concerned about, because I really don't
>>>>wanna break existing http/1.1 playback.
>>>>
>>>> Here are the changes:
>>>>
>>>>- A few additional Lock()s and Unlock()s in CChunkyRes to ensure that
>>>>thread
>>>> safety is maintained.
>>>>- CChunkyRes cursor management addition to SetData(); a new argument
>>>>is
>>>>defaulted
>>>> to zero, and only acted upon if it's non-zero, so existing users of
>>>>CChunkyRes
>>>> continue to work without changes.
>>>>- some http file object initialization was moved from InitObject() to
>>>>_OpenFile()
>>>> since now m_pChunkyRes (and those objects which depend on
>>>>m_pChunkyRes) need
>>>> to know an URL to be properly initialized.
>>>>- http file objects don't create and delete their own CChunkyRes
>>>>objects;
>>>> they use the new CChunkyResMap to acquire and relinquish CChunkyRes
>>>> objects; when each http file object has its own URL, this should
>>>>result in
>>>> exactly the same behavior as before this change.
>>>>- CHTTPFileObject::Seek has additional code to detect whether it has
>>>>partial
>>>> data. Again, this should boil down to the same behavior unless
>>>>several
>>>> http file objects are sharing the same URL.
>>>>- ReadDone() now calls _EnsureThatWeAreReadingWisely() when it didn't
>>>>used
>>>> to. When each http file object has its own URL, this extra call
>>>>should
>>>> have no effect.
>>>>
>>>> Here are the additions:
>>>>
>>>>- CChunkyRes has cursor management functions added. There are times
>>>>when an
>>>> http file object needs to know who was the latest to write to a
>>>>range, and
>>>> adding a simple list of cursor owners and cursors allows this.
>>>>- httpfsys now holds a map which maps URLs to CChunkyRes objects. If
>>>>multiple
>>>> simultaneously-existing http file objects share the same URL (which
>>>>happens
>>>> when we're http-streaming a surestream file), they will now share a
>>>> CChunkyRes.
>>>>- ReadDone() calling _EnsureThatWeAreReadingWisely() -- when multiple
>>>>http
>>>> file objects are sharing the same CChunkyRes, we need this call to
>>>>detect
>>>> when another http file object may have filled in the CChunkyRes
>>>>unbeknownst
>>>> to us. If we detect that we're downloading into an already-valid
>>>>range, we
>>>> stop it.
>>>>
>>>>
>>>>Files Modified:
>>>> common/fileio/chunkres.cpp
>>>> common/fileio/pub/chunkres.h
>>>> filesystem/http/httpfsys.cpp
>>>> filesystem/http/httpfsys.h
>>>>
>>>>Image Size Impact:
>>>> negligibly modest increase in common/fileio and filesystem/http
>>>>
>>>>Heap Size Impact:
>>>> negligibly modest increase in common/fileio and filesystem/http
>>>>
>>>>Platforms/profiles/branches:
>>>> build and fix verified on mac-unix/all-defines/head
>>>> build and fix verified on win32/all-defines/head
>>>>
>>>>
>>>>Diff:
>>>>
>>>>Index: chunkres.cpp
>>>>===================================================================
>>>>RCS file: /cvsroot/common/fileio/chunkres.cpp,v
>>>>retrieving revision 1.14
>>>>diff -u -w -r1.14 chunkres.cpp
>>>>--- chunkres.cpp 15 Jul 2003 20:14:32 -0000 1.14
>>>>+++ chunkres.cpp 25 Feb 2004 20:59:45 -0000
>>>>@@ -602,6 +602,7 @@
>>>>
>>>> ULONG32 CChunkyRes::GetContiguousLength(ULONG32 offset /* = 0 */)
>>>> {
>>>>+ Lock();
>>>> ULONG32 contiguousLength = 0;
>>>> int ndx;
>>>>
>>>>@@ -643,6 +644,7 @@
>>>> }
>>>>
>>>> exit:
>>>>+ Unlock();
>>>> return contiguousLength;
>>>> }
>>>>
>>>>@@ -670,6 +672,7 @@
>>>> //
>>>> HX_RESULT CChunkyRes::GetData(ULONG32 offset, char* buf, ULONG32
>>>>count, ULONG32* actual)
>>>> {
>>>>+ Lock();
>>>> HX_RESULT theErr = HXR_OK;
>>>> int ndx;
>>>>
>>>>@@ -755,12 +758,14 @@
>>>>
>>>> exit:
>>>>
>>>>+ Unlock();
>>>> return theErr;
>>>> }
>>>>
>>>> HX_RESULT
>>>> CChunkyRes::GetContiguousDataPointer(ULONG32 offset, char*& buf,
>>>>ULONG32 count)
>>>> {
>>>>+ Lock();
>>>> HX_RESULT theErr = HXR_OK;
>>>> HX_ASSERT(m_bDiscardUsedData == FALSE && m_bDisableDiskIO ==
>>>>FALSE);
>>>>
>>>>@@ -802,6 +807,7 @@
>>>> ULONG32 chunkAmount = min(DEF_CHUNKYRES_CHUNK_SIZE-chunkOffset,
>>>>count);
>>>>
>>>> theErr =
>>>>pChunk->GetContiguousDataPointer(chunkOffset,buf,chunkAmount);
>>>>+ Unlock();
>>>> return theErr;
>>>> }
>>>>
>>>>@@ -826,8 +832,11 @@
>>>> // HX_RESULT
>>>> // Possible errors include: TBD.
>>>> //
>>>>-HX_RESULT CChunkyRes::SetData(ULONG32 offset, const char* buf,
>>>>ULONG32
>>>>count)
>>>>+
>>>>+HX_RESULT CChunkyRes::SetData(ULONG32 offset, const char* buf,
>>>>ULONG32
>>>>count, void* pOwner /* = NULL */)
>>>> {
>>>>+ Lock();
>>>>+
>>>> HX_RESULT theErr = HXR_OK;
>>>>
>>>> ULONG32 ulFirstChunk = offset/DEF_CHUNKYRES_CHUNK_SIZE;
>>>>@@ -877,8 +886,15 @@
>>>> chunkOffset = 0;
>>>> }
>>>>
>>>>+ // maintain the cursor, if this SetData includes a cursor
>>>>owner.
>>>>+ if (pOwner)
>>>>+ {
>>>>+ SetCursor(pOwner, offset + count);
>>>>+ }
>>>>+
>>>> exit:
>>>>
>>>>+ Unlock();
>>>> return theErr;
>>>> }
>>>>
>>>>@@ -914,6 +930,98 @@
>>>> }
>>>> }
>>>>
>>>>+void
>>>>+CChunkyRes::AddCursor(void* pCursorOwner, ULONG32 ulCursorLocation)
>>>>+{
>>>>+ CursorInfo* pCursorInfo = new CursorInfo;
>>>>+
>>>>+ pCursorInfo->pCursorOwner = pCursorOwner;
>>>>+ pCursorInfo->ulCursorLocation = ulCursorLocation;
>>>>+
>>>>+ m_CursorList.AddTail(pCursorInfo);
>>>>+}
>>>>+
>>>>+void
>>>>+CChunkyRes::SetCursor(void* pCursorOwner, ULONG32 ulCursorLocation)
>>>>+{
>>>>+ LISTPOSITION pos = m_CursorList.GetHeadPosition();
>>>>+ BOOL bDidUpdateCursorInformation = FALSE;
>>>>+
>>>>+ if (pos)
>>>>+ {
>>>>+ do
>>>>+ {
>>>>+ CursorInfo* pCursorInfo =
>>>>(CursorInfo*)m_CursorList.GetNext(pos);
>>>>+
>>>>+ HX_ASSERT(pCursorInfo);
>>>>+
>>>>+ if (pCursorOwner == pCursorInfo->pCursorOwner)
>>>>+ {
>>>>+ pCursorInfo->ulCursorLocation = ulCursorLocation;
>>>>+ bDidUpdateCursorInformation = TRUE;
>>>>+ }
>>>>+ }
>>>>+ while (pos);
>>>>+ }
>>>>+
>>>>+ HX_ASSERT(bDidUpdateCursorInformation);
>>>>+}
>>>>+
>>>>+void
>>>>+CChunkyRes::RemoveCursor(void* pCursorOwner)
>>>>+{
>>>>+ LISTPOSITION pos = m_CursorList.GetHeadPosition();
>>>>+ LISTPOSITION prevPos = NULL;
>>>>+
>>>>+ BOOL bDidRemoveOwner = FALSE;
>>>>+
>>>>+ if (pos)
>>>>+ {
>>>>+ do
>>>>+ {
>>>>+ prevPos = pos;
>>>>+ CursorInfo* pCursorInfo =
>>>>(CursorInfo*)m_CursorList.GetNext(pos);
>>>>+
>>>>+ HX_ASSERT(pCursorInfo);
>>>>+
>>>>+ if (pCursorOwner == pCursorInfo->pCursorOwner)
>>>>+ {
>>>>+ m_CursorList.RemoveAt(prevPos);
>>>>+ bDidRemoveOwner = TRUE;
>>>>+ }
>>>>+ }
>>>>+ while (pos);
>>>>+ }
>>>>+
>>>>+ HX_ASSERT(bDidRemoveOwner);
>>>>+}
>>>>+
>>>>+ULONG32
>>>>+CChunkyRes::CountCursors()
>>>>+{
>>>>+ return m_CursorList.GetCount();
>>>>+}
>>>>+
>>>>+HX_RESULT
>>>>+CChunkyRes::GetNthCursorInformation(int nIndex,
>>>>REF(void*)pCursorOwner, REF(ULONG32)ulCursorLocation)
>>>>+{
>>>>+ LISTPOSITION pos = m_CursorList.FindIndex(nIndex);
>>>>+ HX_ASSERT(pos);
>>>>+
>>>>+ if (!pos)
>>>>+ {
>>>>+ return HXR_UNEXPECTED;
>>>>+ }
>>>>+
>>>>+ CursorInfo* pCursorInfo = (CursorInfo*)m_CursorList.GetAt(pos);
>>>>+ HX_ASSERT(pCursorInfo);
>>>>+
>>>>+ pCursorOwner = pCursorInfo->pCursorOwner;
>>>>+ ulCursorLocation = pCursorInfo->ulCursorLocation;
>>>>+
>>>>+ return HXR_OK;
>>>>+}
>>>>+
>>>>
>>>>////////////////////////////////////////////////////////////////////
>>>>/// //////
>>>> //
>>>> // Method:
>>>>@@ -988,6 +1096,7 @@
>>>>
>>>> HX_ASSERT(m_ChunksMemoryMRU);
>>>> HX_ASSERT(m_ChunksDiskMRU);
>>>>+
>>>> }
>>>>
>>>>
>>>>////////////////////////////////////////////////////////////////////
>>>>/// //////
>>>>Index: pub/chunkres.h
>>>>===================================================================
>>>>RCS file: /cvsroot/common/fileio/pub/chunkres.h,v
>>>>retrieving revision 1.3
>>>>diff -u -w -r1.3 chunkres.h
>>>>--- pub/chunkres.h 4 Mar 2003 04:20:02 -0000 1.3
>>>>+++ pub/chunkres.h 25 Feb 2004 20:59:45 -0000
>>>>@@ -215,7 +215,7 @@
>>>> HX_RESULT SetData
>>>> (
>>>>
>>>>ULONG32 offset, const char* buf,
>>>>-
>>>>ULONG32 count
>>>>+
>>>>ULONG32 count, void* pOwner = NULL
>>>> );
>>>> void Lock()
>>>> {
>>>>@@ -240,6 +240,28 @@
>>>>
>>>> CChunkyRes();
>>>> ~CChunkyRes();
>>>>+
>>>>+ void AddCursor(void* pCursorOwner, ULONG32 ulCursorLocation
>>>>=
>>>>0);
>>>>+ void SetCursor(void* pCursorOwner, ULONG32
>>>>ulCursorLocation);
>>>>+ void RemoveCursor(void* pCursorOwner);
>>>>+ ULONG32 CountCursors();
>>>>+ HX_RESULT GetNthCursorInformation(int nIndex,
>>>>REF(void*)pCursorOwner, REF(ULONG32)ulCursorLocation);
>>>>+
>>>>+private:
>>>>+ struct CursorInfo
>>>>+ {
>>>>+ void* pCursorOwner;
>>>>+ ULONG32 ulCursorLocation;
>>>>+ };
>>>>+
>>>>+
>>>>+ CHXSimpleList m_CursorList;
>>>>+ CHXMapPtrToPtr m_CursorLocationsMap;
>>>> };
>>
>>pCursorOwner needs to be AddRef in AddCursor() and Released in
>>RemoveCursor() to ensure the file object
>>won't be deleted accidently by other components.
>>
>>m_CursorLocationMap is not used. Actually I think it's better to use
>>map instead of list for faster lookup since
>>SetCursor() is called very frequently.
>>
>>I suggest to also define a new method GetCursorInfo() which takes
>>pCursorOwner and returns ulCursorLocation, see reason below.
>>
>>>> ///////////////////////////////////////////////////////////////
>>>>
>>>>Index: httpfsys.cpp
>>>>===================================================================
>>>>RCS file: /cvsroot/filesystem/http/httpfsys.cpp,v
>>>>retrieving revision 1.28
>>>>diff -u -w -r1.28 httpfsys.cpp
>>>>--- httpfsys.cpp 3 Dec 2003 18:38:35 -0000 1.28
>>>>+++ httpfsys.cpp 25 Feb 2004 20:59:59 -0000
>>>>@@ -186,6 +186,9 @@
>>>>
>>>> static INT32 g_nRefCount_http = 0;
>>>>
>>>>+static CChunkyResMap* g_pChunkyResMap = NULL;
>>>>+
>>>>+
>>>> #define WWW_AUTHENTICATION_RECENT_KEY
>>>>"authentication.http.realm.recent"
>>>> #define PROXY_AUTHENTICATION_RECENT_KEY
>>>>"proxy-authentication.http.realm.recent"
>>>>
>>>>@@ -205,6 +208,69 @@
>>>> }
>>>> #endif
>>>>
>>>>+// ChunkyResMap is a manager-type class that is able to re-use the
>>>>same ChunkyRes for different
>>>>+// file objects that use the same url. This happens, for example, if
>>>>you're http-streaming a
>>>>+// surestream file which may have many logical streams pasted one
>>>>after another.
>>>>+
>>>>+CChunkyResMap::CChunkyResMap()
>>>>+{
>>>>+ m_pChunkyResURLMap = new CHXMapStringToOb();
>>>>+}
>>>>+
>>>>+CChunkyResMap::~CChunkyResMap()
>>>>+{
>>>>+ delete m_pChunkyResURLMap;
>>>>+}
>>>>+
>>>>+CChunkyRes*
>>>>+CChunkyResMap::GetChunkyResForURL(const char* pURL, void*
>>>>pCursorOwner)
>>>>+{
>>>>+ CChunkyRes* pChunkyRes = NULL;
>>>>+
>>>>+ if ( !m_pChunkyResURLMap->Lookup( pURL, (void*&)pChunkyRes ) )
>>>>+ {
>>>>+ pChunkyRes = new CChunkyRes();
>>>>+ m_pChunkyResURLMap->SetAt(pURL, pChunkyRes);
>>>>+ }
>>>>+
>>>>+ HX_ASSERT(pChunkyRes);
>>>>+ pChunkyRes->AddCursor(pCursorOwner);
>>>>+
>>>>+ return pChunkyRes;
>>>>+}
>>>>+
>>>>+void
>>>>+CChunkyResMap::RelinquishChunkyRes(CChunkyRes* pChunkyRes, void*
>>>>pCursorOwner)
>>>>+{
>>>>+ if (!pChunkyRes) return;
>>>>+
>>>>+ CChunkyRes* pChunkyResInMap = NULL;
>>>>+ CHXString strURL;
>>>>+
>>>>+ LISTPOSITION pos;
>>>>+ LISTPOSITION prevPos = NULL;
>>>>+
>>>>+ pos = m_pChunkyResURLMap->GetStartPosition();
>>>>+
>>>>+
>>>>+ while (pos)
>>>>+ {
>>>>+ prevPos = pos;
>>>>+ m_pChunkyResURLMap->GetNextAssoc(pos, strURL,
>>>>(void*&)pChunkyResInMap);
>>>>+ if (pChunkyRes == pChunkyResInMap)
>>>>+ {
>>>>+ pChunkyRes->RemoveCursor(pCursorOwner);
>>>>+
>>>>+ if (pChunkyRes->CountCursors() == 0)
>>>>+ {
>>>>+ m_pChunkyResURLMap->RemoveKey(strURL);
>>>>+ delete pChunkyResInMap;
>>>>+ }
>>>>+ }
>>>>+ }
>>>>+}
>>>>+
>>>>+
>>>>
>>>>/
>>>>*********************************************************************
>>>>*** ****
>>>> *
>>>> * Function:
>>>>@@ -228,6 +294,8 @@
>>>> IUnknown** /*OUT*/ ppIUnknown
>>>> )
>>>> {
>>>>+ g_pChunkyResMap = new CChunkyResMap();
>>>>+
>>>> // Do NOT check for expiration. Needed for Auto Upgrade.
>>>>
>>>> *ppIUnknown = (IUnknown*)(IHXPlugin*)new CHTTPFileSystem();
>>>>@@ -271,6 +339,9 @@
>>>> g_pCacheEntry = NULL;
>>>> }
>>>>
>>>>+ delete g_pChunkyResMap;
>>>>+ g_pChunkyResMap = NULL;
>>>>+
>>>> return HXR_OK;
>>>> }
>>>>
>>>>@@ -1049,30 +1120,6 @@
>>>> }
>>>> }
>>>>
>>>>- m_pChunkyRes = new CChunkyRes;
>>>>-
>>>>- #ifdef CREATE_LOG_DUMP
>>>>- FILE *fp = fopen(LOG_DUMP_FILE, "a+");
>>>>- if (fp)
>>>>- {
>>>>- fprintf(fp, "NewChunky %lx\n", m_pChunkyRes);
>>>>- fclose(fp);
>>>>- }
>>>>- #endif
>>>>-
>>>>- if (m_bOnServer)
>>>>- {
>>>>- m_pChunkyRes->DisableDiskIO();
>>>>- }
>>>>-
>>>>-#if defined(HELIX_FEATURE_HTTP_GZIP)
>>>>- m_pDecoder = new CDecoder;
>>>>- if (m_pDecoder && m_pChunkyRes)
>>>>- {
>>>>- m_pDecoder->SetOutputSink(m_pChunkyRes);
>>>>- }
>>>>-#endif
>>>>-
>>>> m_pCallback = HTTPFileObjCallback::CreateObject();
>>>> if (m_pCallback)
>>>> {
>>>>@@ -1240,6 +1287,12 @@
>>>> m_pPAC->AbortProxyInfo(this);
>>>> }
>>>>
>>>>+ g_pChunkyResMap->RelinquishChunkyRes(m_pChunkyRes, this);
>>>>+ m_pChunkyRes = NULL;
>>>>+
>>>> HX_RELEASE(m_pFileSystem);
>>>> HX_RELEASE(m_pRequest);
>>>> HX_RELEASE(m_pRequestHeadersOrig);
>>>>@@ -1292,7 +1345,6 @@
>>>> #if defined(HELIX_FEATURE_HTTP_GZIP)
>>>> HX_DELETE(m_pDecoder);
>>>> #endif
>>>>- HX_DELETE(m_pChunkyRes);
>>>>
>>>> if (m_pChunkedEncoding)
>>>> {
>>>>@@ -1567,7 +1619,42 @@
>>>> m_pFileResponse->SeekDone(HXR_CANCELLED);
>>>> }
>>>>
>>>>- if (m_pChunkyRes->HasPartialData(1, m_ulCurrentReadPosition))
>>>>+ // bobclark we need to see if we're currently reading
>>>>+ // into the range to which we're seeking. With changes
>>>>+ // to support multiple http file objects writing to
>>>>+ // the same chunky res, we need to double-check to
>>>>+ // ensure that it's not a different http file object
>>>>+ // that's doing the writing. It will confuse us later
>>>>+ // on if it's relying on a different http object's
>>>>+ // cursor.
>>>>+ BOOL bHasPartialData = FALSE;
>>>>+ ULONG32 ulContiguousBytes =
>>>>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition);
>>>>+ if (ulContiguousBytes > 0)
>>>>+ {
>>>>+ int max = m_pChunkyRes->CountCursors();
>>>>+ int ndx;
>>>>+ void* pCursorOwner;
>>>>+ ULONG32 ulCursorLocation;
>>>>+
>>>>+ for (ndx = 0; ndx < max; ndx++)
>>>>+ {
>>>>+ m_pChunkyRes->GetNthCursorInformation(ndx, pCursorOwner,
>>>>ulCursorLocation);
>>>>+
>>>>+ if (pCursorOwner == this)
>>>>+ {
>>>>+ if ( (ulCursorLocation >= m_ulCurrentReadPosition)
>>>>+ && (ulCursorLocation <= m_ulCurrentReadPosition
>>>>+
>>>>ulContiguousBytes) )
>>>>+ {
>>>>+ // if my cursor is currently filling in data in
>>>>this
>>>>+ // correct range, I can be confident that it
>>>>does
>>>>have
>>>>+ // partial data.
>>>>+ bHasPartialData = TRUE;
>>>>+ }
>>>>+ }
>>>>+ }
>>>>+ }
>>>>+
>>
>>We can use the new method GetCursorInfo() above
>>
>>>>+ if (bHasPartialData)
>>>> {
>>>> // Don't call SeekDone until we are actaully done with the
>>>>seek
>>>> if (m_bSupportsByteRanges)
>>>>@@ -1577,7 +1664,9 @@
>>>> // m_bByteRangeSeekPending means we reconnecting the
>>>>socket
>>>> // a new location.
>>>> if (m_bByteRangeSeekPending)
>>>>+ {
>>>> m_bSeekPending = TRUE;
>>>>+ }
>>>> else
>>>> {
>>>> #ifdef CREATE_LOG_DUMP
>>>>@@ -2032,6 +2121,45 @@
>>>>
>>>>
>>>>/
>>>>********************************************************************* ***
>>>> * Method:
>>>>+ * Private interface::InitializeChunkyRes
>>>>+ * Purpose:
>>>>+ * This encapsulates initialization of the chunky res and
>>>>associated
>>>>+ * objects
>>>>+ */
>>>>+HX_RESULT CHTTPFileObject::_InitializeChunkyRes(const char* url)
>>>>+{
>>>>+ if (!m_pChunkyRes)
>>>>+ {
>>>>+ m_pChunkyRes = g_pChunkyResMap->GetChunkyResForURL(url,
>>>>this);
>>>>+
>>>>+#ifdef CREATE_LOG_DUMP
>>>>+ FILE *fp = fopen(LOG_DUMP_FILE, "a+");
>>>>+ if (fp)
>>>>+ {
>>>>+ fprintf(fp, "NewChunky %lx\n", m_pChunkyRes);
>>>>+ fclose(fp);
>>>>+ }
>>>>+#endif
>>>>+
>>>>+ if (m_bOnServer)
>>>>+ {
>>>>+ m_pChunkyRes->DisableDiskIO();
>>>>+ }
>>>>+
>>>>+#if defined(HELIX_FEATURE_HTTP_GZIP)
>>>>+ m_pDecoder = new CDecoder;
>>>>+ if (m_pDecoder && m_pChunkyRes)
>>>>+ {
>>>>+ m_pDecoder->SetOutputSink(m_pChunkyRes);
>>>>+ }
>>>>+#endif
>>>>+
>>>>+ }
>>>>+ return HXR_OK;
>>>>+}
>>>>+
>>>>+/
>>>>********************************************************************* ***
>>>>+ * Method:
>>>> * Private interface::OpenFile
>>>> * Purpose:
>>>> * This common method is used from Init() and
>>>>GetFileObjectFromPool()
>>>>@@ -2048,6 +2176,8 @@
>>>> IHXBuffer* pProxyPort = NULL;
>>>> IHXProxyManager* pProxyManager = NULL;
>>>>
>>>>+ _InitializeChunkyRes(url);
>>>>+
>>>> LOG("_OpenFile");
>>>> LOGX((szDbgTemp, " URL='%s'", url));
>>>> // Make local copy of url
>>>>@@ -3488,8 +3619,15 @@
>>>> #endif
>>>>
>>>> m_pChunkyRes->SetData(m_nContentRead,
>>>>- (char*)pBuffer->GetBuffer(), size);
>>>>+ (char*)pBuffer->GetBuffer(), size, this);
>>>> m_nContentRead += size;
>>>>+
>>>>+ // xxxbobclark it's possible -- especially if we
>>>>have
>>>>several
>>>>+ // http file objects sharing a single chunky res
>>>>(like
>>>>if we're
>>>>+ // accessing a surestream file) -- that we might
>>>>stumble onto
>>>>+ // parts of the file that we've already downloaded;
>>>>by
>>>>calling
>>>>+ // _Ensure() here we can avoid redundant
>>>>downloading.
>>>>+ _EnsureThatWeAreReadingWisely();
>>>> }
>>>>
>>>> if(m_bKnowContentSize && m_nContentRead >=
>>>>m_nContentSize)
>>>>@@ -4184,9 +4322,57 @@
>>>> // then seeked backwards, and has now finished
>>>>reading
>>>> // up to the already-valid place. So we need to
>>>> // kick-start reading way down the line.
>>>>+
>>>>+ // xxxbobclark now here's the problem: we can easily
>>>>end up with two separate
>>>>+ // http file objs writing into the same chunky res
>>>>here. If they're writing into
>>>>+ // the same range we'll end up with "leapfrogging"
>>>>cursors, where each iteration
>>>>+ // will do a whole nuther byte range seek to the
>>>>last
>>>>set byte that the other
>>>>+ // guy just wrote. (This is kind of fun to watch in
>>>>action, but less than
>>>>+ // optimal in the real world.) What I'd rather do
>>>>instead is see if another obj
>>>>+ // is actively writing into this range and if so
>>>>I'll
>>>>just stop reading.
>>>>+
>>>>+ BOOL bSomebodyElseOwnsThisRange = FALSE;
>>>>+ {
>>>>+ int max = m_pChunkyRes->CountCursors();
>>>>+ int ndx;
>>>>+ for (ndx = 0; ndx < max; ndx++)
>>>>+ {
>>>>+ void* pOwner;
>>>>+ ULONG32 ulCursorLocation;
>>>>+ m_pChunkyRes->GetNthCursorInformation(ndx,
>>>>pOwner, ulCursorLocation);
>>>>+
>>>>+ if (ulCursorLocation ==
>>>>m_ulCurrentReadPosition + ulLength)
>>>>+ {
>>>>+ if (pOwner != this)
>>>>+ {
>>>>+ bSomebodyElseOwnsThisRange = TRUE;
>>>>+ }
>>>>+ }
>>>>+ }
>>>>+ }
>>>>+
>>>>+ if (bSomebodyElseOwnsThisRange)
>>>>+ {
>>>>+ m_pChunkyRes->SetCursor(this, 0);
>>>>+ // If there is a pending callback, be sure to
>>>>remove it!
>>>>+ // (there might be the connection timeout
>>>>callback
>>>>pending)
>>>>+ if (m_pCallback &&
>>>>+ m_pCallback->m_bCallbackPending &&
>>>>+ m_pCallback->m_ulPendingCallbackID &&
>>>>+ m_pScheduler)
>>>>+ {
>>>>+
>>>>m_pScheduler->Remove(m_pCallback->m_ulPendingCallbackID);
>>>>+ m_pCallback->m_bCallbackPending = FALSE;
>>>>+ }
>>>>+
>>>>+ HX_RELEASE(m_pSocket);
>>>>+ }
>>>>+ else
>>>>+ {
>>
>>So we will stop reading from the socket, but we should still keep
>>reading from the ChunkyRes to satisfy
>>the read request from the file format, correct?
>>
>>>> _HandleByteRangeSeek(m_ulCurrentReadPosition +
>>>>ulLength);
>>>> }
>>>> }
>>>>+ }
>>>> else
>>>> {
>>>> // XXXbobclark OK, it's plausible that something could be
>>>>@@ -4200,8 +4386,8 @@
>>>> // seek.
>>>>
>>>> // This is the number of contigous bytes we have
>>>>- ULONG32 ulLength =
>>>>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition) +
>>>>m_ulCurrentReadPosition;
>>>>- _HandleByteRangeSeek(ulLength);
>>>>+ ULONG32 ulLength =
>>>>m_pChunkyRes->GetContiguousLength(m_ulCurrentReadPosition);
>>>>+ _HandleByteRangeSeek(m_ulCurrentReadPosition +
>>>>ulLength);
>>
>>The above changes don't seems to be related to this multiple
>>downloads' fix?
>>
>>Can you also verify the same HTTP URL defined multiple times in RAM
>>and SMIL(within par) are
>>working properly?
>>
>>-->Henry
>>
>>>> //HX_ASSERT(!"Reading BEHIND the current seek position");
>>>> }
>>>>@@ -4871,7 +5057,7 @@
>>>>
>>>> m_pChunkyRes->SetData(m_nContentRead,
>>>> (const char*)pBuffer->GetBuffer() +
>>>>ulHeaderLength,
>>>>- nContentLen);
>>>>+ nContentLen, this);
>>>> m_nContentRead += nContentLen;
>>>>
>>>> }
>>>>@@ -5733,7 +5919,7 @@
>>>> #endif
>>>>
>>>> // copy the data from the cache
>>>>- m_pChunkyRes->SetData(0, (const char *)dbtContent.data,
>>>>dbtContent.size);
>>>>+ m_pChunkyRes->SetData(0, (const char *)dbtContent.data,
>>>>dbtContent.size, this);
>>>>
>>>> m_bCached = TRUE;
>>>> // These are values expected to be set when data is
>>>>downloaded
>>>>@@ -7166,7 +7352,7 @@
>>>> }
>>>> #endif
>>>>
>>>>- m_pChunkyRes->SetData(m_nContentRead,
>>>>pChunkedEncoding->buf, pChunkedEncoding->size);
>>>>+ m_pChunkyRes->SetData(m_nContentRead,
>>>>pChunkedEncoding->buf, pChunkedEncoding->size, this);
>>>> m_nContentRead += pChunkedEncoding->size;
>>>>
>>>> memset(pChunkedEncoding->buf, 0, MAX_CHUNK_SIZE);
>>>>Index: httpfsys.h
>>>>===================================================================
>>>>RCS file: /cvsroot/filesystem/http/httpfsys.h,v
>>>>retrieving revision 1.5
>>>>diff -u -w -r1.5 httpfsys.h
>>>>--- httpfsys.h 8 Oct 2003 16:49:10 -0000 1.5
>>>>+++ httpfsys.h 25 Feb 2004 20:59:59 -0000
>>>>@@ -73,6 +73,7 @@
>>>> class CChunkyRes;
>>>> class CHXSimpleList;
>>>> class CDecoder;
>>>>+class CHXMapStringToOb;
>>>>
>>>> class HTTPTCPResponse;
>>>> class HTTPFileObjCallback;
>>>>@@ -96,6 +97,21 @@
>>>> char* buf;
>>>> } HTTPChunkedEncoding;
>>>>
>>>>+
>>>>+class CChunkyResMap
>>>>+{
>>>>+public:
>>>>+ CChunkyResMap::CChunkyResMap();
>>>>+ virtual CChunkyResMap::~CChunkyResMap();
>>>>+ CChunkyRes* GetChunkyResForURL(const char* pURL, void*
>>>>pCursorOwner);
>>>>+ void RelinquishChunkyRes(CChunkyRes* pChunkyRes, void*
>>>>pCursorOwner);
>>>>+
>>>>+private:
>>>>+ CHXMapStringToOb* m_pChunkyResURLMap;
>>>>+};
>>>>+
>>>>+
>>>>+
>>>>
>>>>////////////////////////////////////////////////////////////////////
>>>>/// //////
>>>> //
>>>> // Class:
>>>>@@ -389,6 +405,8 @@
>>>>
>>>> HX_RESULT _ReOpen();
>>>>
>>>>+ HX_RESULT _InitializeChunkyRes(const char* url);
>>>>+
>>>>
>>>>/
>>>>********************************************************************* ***
>>>> * Method:
>>>> * Private interface::_OpenFile
>>>
>>>
>>>_______________________________________________
>>>Filesystem-dev mailing list
>>>Filesystem-dev at lists.helixcommunity.org
>>>http://lists.helixcommunity.org/mailman/listinfo/filesystem-dev
>>
>
>
>_______________________________________________
>Filesystem-dev mailing list
>Filesystem-dev at lists.helixcommunity.org
>http://lists.helixcommunity.org/mailman/listinfo/filesystem-dev