[prev in list] [next in list] [prev in thread] [next in thread]
List: helix-datatype-dev
Subject: [datatype-dev] CR: crash/hang on exit with dtdrplin
From: Jamie Gordon <jgordon () real ! com>
Date: 2011-07-10 18:54:07
Message-ID: 4E19F54F.9090800 () real ! com
[Download RAW message or body]
This is a MIME-formatted message. If you see this text it means that your
E-mail software does not support MIME-formatted messages.
Synopsis
========
Fixes a crash one exit and occasional hang on close with dtddrplin.
NOTE: prod14 only until the necessary updates are ready for to the
non-windows threaded callback manager and I've had a chance to
test synchronous mode.
Branches: PRODUCER_14_0_RN
Suggested Reviewer: anyone
Description
===========
dtdrplin is calling the response's OnTerminate way too soon - unless
Close is called in Stopped state, in which case it never calls back to
OnTerminate.
OnTerminate is being called on OnTermination from the source. But that
is always too soon - not only is the dtdrplin thread not terminated
yet, but its network thread (if applicable), MediaEngine and associated
threads, etc. are not closed and the dtdr engine has not even stopped,
closed the fileformat, etc.
This naturally causes problems - for instance if the calling process is
going to exit when the dtdriver is done, it has no idea when it is safe
to do so. OnTerminate needs to happen when it really is completely
terminated!
In cases where it is in Stopped state when the caller tries to close,
there is no OnTermination to be done, so OnTerminate does not ever
happen.
Also, calling OnTerminate from OnTermination clearly renders moot all
the special handling going on that is trying to allow the caller to
re-open for another file/URL, but I did not test whether this change
completely fixes that case.
Change is to wait until the engine's Drive has returned (which means
the engine is stopped) and if we are closing or get a close event as
the next input (rather than another Open->Drive), signal back to the
main calling thread to shut everything down, close the dtdriver thread,
and call back to the response. Or in case Close is called while already
Stopped, then do those things right there.
Files Affected
==============
datatype/tools/dtdriver/dtdrplin/cbmgr.cpp
datatype/tools/dtdriver/dtdrplin/cbmgr.h
datatype/tools/dtdriver/dtdrplin/datatypedr.cpp
datatype/tools/dtdriver/dtdrplin/messages.h
datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.cpp
datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.h
datatype/tools/dtdriver/dtdrplin/pub/datatypedr.h
Testing Performed
=================
Close during 'playback'
Natural close at end of stream
Engine natural close on headers ready in ProcessHeadersOnly case
Platforms Tested: win32-i386-vc9
Build verified: win32-i386-vc9
QA Hints
===============
Test stopping during decode, stopping during initialization/
timeout/retry, running to stream end, and -psi.
["dtdrterm.diff" (text/plain)]
Index: datatype/tools/dtdriver/dtdrplin/cbmgr.cpp
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/cbmgr.cpp,v
retrieving revision 1.2
diff -u -w -r1.2 cbmgr.cpp
--- datatype/tools/dtdriver/dtdrplin/cbmgr.cpp 17 Apr 2009 20:19:54 -0000 1.2
+++ datatype/tools/dtdriver/dtdrplin/cbmgr.cpp 10 Jul 2011 18:11:35 -0000
@@ -73,6 +73,15 @@
return HXR_OK;
}
+HX_RESULT CallbackManagerBase::Terminate(HX_RESULT status)
+{
+ if(m_pCvtr)
+ {
+ return m_pCvtr->CB_OnTerminate(status);
+ }
+ return HXR_OK;
+}
+
HX_RESULT CallbackManagerBase::Cvtr_OnFileHeader(HX_RESULT status, IHXValues* pValues)
{
if(m_pCvtr)
@@ -118,6 +127,15 @@
return HXR_OK;
}
+HX_RESULT CallbackManagerBase::Cvtr_OnTerminate(HX_RESULT status)
+{
+ if(m_pCvtr)
+ {
+ return m_pCvtr->CB_OnTerminate(status);
+ }
+ return HXR_OK;
+}
+
IHXMutex* CallbackManagerBase::GetThrottleMutex()
{
if(m_pCvtr)
Index: datatype/tools/dtdriver/dtdrplin/cbmgr.h
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/cbmgr.h,v
retrieving revision 1.2
diff -u -w -r1.2 cbmgr.h
--- datatype/tools/dtdriver/dtdrplin/cbmgr.h 17 Apr 2009 20:19:54 -0000 1.2
+++ datatype/tools/dtdriver/dtdrplin/cbmgr.h 10 Jul 2011 18:11:35 -0000
@@ -52,6 +52,7 @@
*/
virtual HX_RESULT Close();
virtual HX_RESULT ShutDown();
+ virtual HX_RESULT Terminate(HX_RESULT status);
protected:
/*
@@ -62,6 +63,7 @@
HX_RESULT Cvtr_OnStreamDone(HX_RESULT status, UINT16 unStreamNumber);
HX_RESULT Cvtr_OnPacket(HX_RESULT status, IHXPacket* pPacket);
HX_RESULT Cvtr_OnTermination(HX_RESULT status);
+ HX_RESULT Cvtr_OnTerminate(HX_RESULT status);
CDataTypeConverter* GetConverter() { return m_pCvtr; }
IHXMutex* GetThrottleMutex();
Index: datatype/tools/dtdriver/dtdrplin/datatypedr.cpp
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/datatypedr.cpp,v
retrieving revision 1.28.2.1
diff -u -w -r1.28.2.1 datatypedr.cpp
--- datatype/tools/dtdriver/dtdrplin/datatypedr.cpp 3 May 2011 00:53:01 -0000 1.28.2.1
+++ datatype/tools/dtdriver/dtdrplin/datatypedr.cpp 10 Jul 2011 18:11:35 -0000
@@ -725,12 +725,6 @@
// Called from invalid state
ret = HXR_UNEXPECTED;
}
-
- if (ret == HXR_OK)
- {
- // Callback to signal that we have stopped.
- CB_OnTermination(HXR_CLOSED);
- }
}
return ret;
@@ -752,44 +746,74 @@
{
HX_RESULT ret = HXR_OK;
- if (m_pStateMutex)
- {
- m_pStateMutex->Lock();
- if (m_state == State_Closed)
- {
- m_pStateMutex->Unlock();
- return HXR_OK;
- }
// call not allowed in the middle of file-operation in sync mode
if(m_state != State_Stopped && m_bSynchronousMode)
{
- m_pStateMutex->Unlock();
return HXR_UNEXPECTED;
}
+
+ if (m_pStateMutex)
+ {
+ m_pStateMutex->Lock();
+ if (m_state != State_Closed)
+ {
m_bClosing = TRUE;
+ }
+ switch (m_state)
+ {
+ case State_Driving:
+ case State_Paused:
+ {
m_pStateMutex->Unlock();
// Stop the current operation
- Stop();
+ // Shutdown and cleanup will be completed after driver is
+ // complete and closed and our thread func has ended
+ ret = Stop();
- // Shutdown and cleanup only if we are still closing, since
- // the callback in Stop() could have started a new job. For
- // instance, CB_OnTermination could call the OnTermination
- // method of the source input, which could in turn call Open()
- // to start a new job. Open calls Close again to shutdown the
- // current thread. m_bClosing then gets reset to FALSE in
- // CleanUp, and a new thread is created in Open. When we return
- // from Stop, m_bClosing will be FALSE, and the following code
- // will not get executed since a new thread has already started.
- if (m_bClosing)
+ if (m_pDriveEvent)
{
+ m_pDriveEvent->SignalEvent();
+ }
+
+ if (SUCCEEDED(ret))
+ {
+ break;
+ }
+
+ // in case Stop failed, fall through to Stopped case
+ }
+
+ case State_Stopped:
+ {
+ m_pStateMutex->Unlock();
+
// Shutdown the internal thread - will wait until thread exits.
ret = ShutDown();
- CleanUp();
// Thread has exited. Set state to closed.
m_state = State_Closed;
LOG_STRING("State_Closed");
+
+ if (m_pDTDResponse)
+ {
+ m_pDTDResponse->OnTerminate(ret);
+ }
+
+ CleanUp();
+
+ break;
+ }
+
+ default: // Closed, Closing, Stopping
+ {
+ m_pStateMutex->Unlock();
+ if (m_pDriveEvent)
+ {
+ m_pDriveEvent->SignalEvent();
+ }
+ break;
+ }
}
}
@@ -815,6 +839,8 @@
HX_RESULT
CDataTypeConverter::ShutDown()
{
+ HX_RELEASE(m_pFFDriver);
+
if (m_pCBMgr)
{
m_pCBMgr->ShutDown();
@@ -872,12 +898,6 @@
HX_RELEASE(m_pFFDriverContext);
HX_RELEASE(m_pClientContext);
- if (m_pFFDriver)
- {
- m_pFFDriver->Close();
- HX_RELEASE(m_pFFDriver);
- }
-
m_bClosing = FALSE;
Reset();
@@ -1210,14 +1230,10 @@
}
// Termination message should always be sent (even on error).
- if (!m_bTerminationSent)
- {
- // Send termination event if error occurred.
- if (m_pCBMgr)
+ if (!m_bTerminationSent && m_pCBMgr)
{
m_pCBMgr->OnTermination(retVal);
}
- }
m_pStateMutex->Lock();
m_state = State_Stopped;
@@ -1231,7 +1247,7 @@
}
else
{
- // Wait until signalled to drive again.
+ // Wait until signalled to drive again or to close
if (!m_bClosing)
{
retVal = m_pDriveEvent->Wait(ALLFS);
@@ -1239,6 +1255,21 @@
}
}
+ HX_RELEASE(m_pFFDriver);
+
+ m_pStateMutex->Lock();
+ m_state = State_Closing;
+ LOG_STRING("State_Stopped");
+ m_pStateMutex->Unlock();
+
+ if (m_pCBMgr)
+ {
+ // Signal the callback manager to get back on the calling thread
+ // in order to close this thread (the internal thread) and signal
+ // back to the response object OnTerminate
+ m_pCBMgr->Terminate(retVal);
+ }
+
return retVal;
}
@@ -1476,14 +1507,30 @@
m_pSourceInputSink->OnTermination(status);
}
+ return ret;
+}
+
+HX_RESULT
+CDataTypeConverter::CB_OnTerminate(HX_RESULT status)
+{
+ HX_ASSERT(m_state == State_Closing);
+
+ // Shutdown the internal thread - will wait until thread exits.
+ ShutDown();
+
+ // Thread has exited. Set state to closed.
+ m_state = State_Closed;
+ LOG_STRING("State_Closed");
+
if (m_pDTDResponse)
{
m_pDTDResponse->OnTerminate(status);
}
- return ret;
-}
+ CleanUp();
+ return HXR_OK;
+}
/************************************************************************
*
Index: datatype/tools/dtdriver/dtdrplin/messages.h
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/messages.h,v
retrieving revision 1.3
diff -u -w -r1.3 messages.h
--- datatype/tools/dtdriver/dtdrplin/messages.h 19 Aug 2005 07:36:40 -0000 1.3
+++ datatype/tools/dtdriver/dtdrplin/messages.h 10 Jul 2011 18:11:35 -0000
@@ -43,6 +43,7 @@
#define HXMSG_ON_STREAM_DONE (WM_USER + 103) /*Async Read Notification*/
#define HXMSG_ON_PACKET (WM_USER + 104) /*Async Write Notification*/
#define HXMSG_ON_TERMINATION (WM_USER + 105) /*Async Detach Notification*/
+#define HXMSG_ON_TERMINATE (WM_USER + 106) /*Async thread done notification*/
#define HXMSG_QUIT (WM_USER + 200) /* Exit from the thread */
@@ -54,7 +55,7 @@
#define HXMSG_ASYNC_STREAM_DONE 103 /*Async Read Notification*/
#define HXMSG_ASYNC_PACKET 104 /*Async Write Notification*/
#define HXMSG_ASYNC_TERMINATION 105 /*Async Detach Notification*/
-
+#define HXMSG_ON_TERMINATE 106 /*Async thread done notification*/
#define HXMSG_QUIT 200 /* Exit from the thread */
Index: datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.cpp
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.cpp,v
retrieving revision 1.6.26.2
diff -u -w -r1.6.26.2 thrdcbmgr.cpp
--- datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.cpp 3 May 2011 00:53:01 -0000 1.6.26.2
+++ datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.cpp 10 Jul 2011 18:11:35 -0000
@@ -128,6 +128,25 @@
return CallbackManagerBase::Close();
}
+HX_RESULT
+CThreadCBMgr::Terminate(HX_RESULT status)
+{
+ // to be called from internal thread when it's done
+ // so we can get back on to the calling thread,
+ // close this thread, and let the calling thread
+ // know we are really closed
+ if (!m_bShutDown)
+ {
+ // there should not be any messages outstanding!!
+ m_pPrevMessageFinished->Wait(ALLFS);
+ }
+ if (!m_bShutDown)
+ {
+ ::PostMessage(m_hWnd, HXMSG_ON_TERMINATE, (WPARAM)status, 0);
+ }
+ return HXR_OK;
+}
+
// Workhorse construction method
BOOL CThreadCBMgr::Create()
{
@@ -213,6 +232,10 @@
return (pThis->AsyncOnPacket(wParam, lParam));
case HXMSG_ON_TERMINATION:
return (pThis->AsyncOnTermination(wParam, lParam));
+
+ case HXMSG_ON_TERMINATE:
+ DestroyWindow(hWnd);
+ return (pThis->AsyncOnTerminate(wParam, lParam));
default:
break;
}
@@ -319,6 +342,23 @@
return HXR_OK;
}
+LRESULT
+CThreadCBMgr::AsyncOnTerminate(WPARAM wParam, LPARAM lParam)
+{
+ m_pMessageRecieved->SignalEvent();
+
+ // There had better not be anything waiting at this point!
+ // signal now just incase, I guess.
+ if (m_pPrevMessageFinished)
+ {
+ m_pPrevMessageFinished->SignalEvent();
+ }
+
+ Cvtr_OnTerminate((HX_RESULT) wParam);
+
+ return HXR_OK;
+}
+
/*
* IHXSourceInput methods
*/
Index: datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.h
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.h,v
retrieving revision 1.3
diff -u -w -r1.3 thrdcbmgr.h
--- datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.h 5 Nov 2007 22:11:35 -0000 1.3
+++ datatype/tools/dtdriver/dtdrplin/platform/win/thrdcbmgr.h 10 Jul 2011 18:11:35 -0000
@@ -55,6 +55,7 @@
*/
virtual HX_RESULT Close();
virtual HX_RESULT ShutDown();
+ virtual HX_RESULT Terminate(HX_RESULT status);
/*
* IUnknown methods
@@ -144,6 +145,7 @@
LRESULT AsyncOnStreamDone(WPARAM wParam, LPARAM lParam);
LRESULT AsyncOnPacket(WPARAM wParam, LPARAM lParam);
LRESULT AsyncOnTermination(WPARAM wParam, LPARAM lParam);
+ LRESULT AsyncOnTerminate(WPARAM wParam, LPARAM lParam);
};
#endif // _THRDCBMGR_H_
Index: datatype/tools/dtdriver/dtdrplin/pub/datatypedr.h
===================================================================
RCS file: /cvsroot/datatype/tools/dtdriver/dtdrplin/pub/datatypedr.h,v
retrieving revision 1.13
diff -u -w -r1.13 datatypedr.h
--- datatype/tools/dtdriver/dtdrplin/pub/datatypedr.h 4 Sep 2009 16:22:46 -0000 1.13
+++ datatype/tools/dtdriver/dtdrplin/pub/datatypedr.h 10 Jul 2011 18:11:35 -0000
@@ -266,6 +266,7 @@
HX_RESULT CB_OnStreamDone(HX_RESULT status, UINT16 unStreamNumber);
HX_RESULT CB_OnPacket(HX_RESULT status, IHXPacket* pPacket);
HX_RESULT CB_OnTermination(HX_RESULT status);
+ HX_RESULT CB_OnTerminate(HX_RESULT status);
void Reset(void);
void HandleInProgressTimeoutCallback(void);
_______________________________________________
Datatype-dev mailing list
Datatype-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic