Re: [win-pv-devel] [PATCH 14/20] Protect active device with critical section

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: win-pv-devel [mailto:win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On
> Behalf Of Owen Smith
> Sent: 24 May 2016 15:21
> To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Owen Smith
> Subject: [win-pv-devel] [PATCH 14/20] Protect active device with critical
> section
> 
> This should prevent race conditions due to device removal while
> currently attempting operations. Will ensure the m_dev pointer
> is not removed during a OnSuspend or OnShutdown operation
>

Possibly a candidate for folding, but leaving it broken out is illustrative 
so...

> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Acked-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
>  src/liteagent/LiteAgent.cpp | 32 +++++++++++++++++++++++++++++++-
>  src/liteagent/LiteAgent.h   |  1 +
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/liteagent/LiteAgent.cpp b/src/liteagent/LiteAgent.cpp
> index b61967f..db6c3b9 100644
> --- a/src/liteagent/LiteAgent.cpp
> +++ b/src/liteagent/LiteAgent.cpp
> @@ -42,6 +42,24 @@
>  #define XENTOOLS_INSTALL_REG_KEY64
> "SOFTWARE\\Wow6432Node\\Citrix\\XenTools"
>  #endif
> 
> +class CCritSec
> +{
> +public:
> +    CCritSec(LPCRITICAL_SECTION crit);
> +    ~CCritSec();
> +private:
> +    LPCRITICAL_SECTION m_crit;
> +};
> +
> +CCritSec::CCritSec(LPCRITICAL_SECTION crit) : m_crit(crit)
> +{
> +    EnterCriticalSection(m_crit);
> +}
> +CCritSec::~CCritSec()
> +{
> +    LeaveCriticalSection(m_crit);
> +}
> +
>  int __stdcall WinMain(HINSTANCE hInstance, HINSTANCE ignore, LPSTR
> lpCmdLine, int nCmdShow)
>  {
>      if (strlen(lpCmdLine) != 0) {
> @@ -169,7 +187,9 @@ CLiteAgent::CLiteAgent() :
> 
>      m_svc_stop = CreateEvent(FALSE, NULL, NULL, FALSE);
>      m_shutdown = CreateEvent(FALSE, NULL, NULL, FALSE);
> -    m_suspend  = CreateEvent(FALSE, NULL, NULL, FALSE);
> +    m_suspend  = CreateEvent(FALSE, NULL, NULL, FALSE);
> +
> +    InitializeCriticalSection(&m_crit);
>  }
> 
>  CLiteAgent::~CLiteAgent()
> @@ -177,6 +197,8 @@ CLiteAgent::~CLiteAgent()
>      CloseHandle(m_svc_stop);
>      CloseHandle(m_shutdown);
>      CloseHandle(m_suspend);
> +
> +    DeleteCriticalSection(&m_crit);
>  }
> 
>  /*virtual*/ CDevice* CLiteAgent::Create(const wchar_t* path)
> @@ -188,6 +210,8 @@ CLiteAgent::~CLiteAgent()
>  /*virtual*/ void CLiteAgent::OnDeviceAdded(CDevice* dev)
>  {
>      CLiteAgent::Log("OnDeviceAdded(%ws)\n", dev->Path());
> +
> +    CCritSec crit(&m_crit);
>      if (m_dev == NULL) {
>          m_dev = (CXenIfaceItf*)dev;
>          // setting active device
> @@ -210,6 +234,8 @@ CLiteAgent::~CLiteAgent()
>  /*virtual*/ void CLiteAgent::OnDeviceRemoved(CDevice* dev)
>  {
>      CLiteAgent::Log("OnDeviceRemoved(%ws)\n", dev->Path());
> +
> +    CCritSec crit(&m_crit);
>      if ((CXenIfaceItf*)dev == m_dev) {
>          // active device removed
>          CLiteAgent::Log("Active Device Removed\n");
> @@ -273,6 +299,8 @@ bool CLiteAgent::ServiceMainLoop()
> 
>  void CLiteAgent::OnShutdown()
>  {
> +    CCritSec crit(&m_crit);
> +
>      if (m_dev == NULL)
>          return;
>      CLiteAgent::Log("OnShutdown(%ws)\n", m_dev->Path());
> @@ -286,6 +314,8 @@ void CLiteAgent::OnShutdown()
> 
>  void CLiteAgent::OnSuspend()
>  {
> +    CCritSec crit(&m_crit);
> +
>      if (m_dev == NULL)
>          return;
>      CLiteAgent::Log("OnSuspend(%ws)\n", m_dev->Path());
> diff --git a/src/liteagent/LiteAgent.h b/src/liteagent/LiteAgent.h
> index 97c6514..e3a2573 100644
> --- a/src/liteagent/LiteAgent.h
> +++ b/src/liteagent/LiteAgent.h
> @@ -87,6 +87,7 @@ private: // service support
>      HANDLE                  m_shutdown;
>      HANDLE                  m_suspend;
> 
> +    CRITICAL_SECTION        m_crit;
>      CDeviceList             m_devs;
>      CXenIfaceItf*           m_dev;
>      void*                   m_ctxt_shutdown;
> --
> 1.9.4.msysgit.1
> 
> 
> _______________________________________________
> win-pv-devel mailing list
> win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel
_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/win-pv-devel