[PATCH 2/5] xenagent: Let CXenIfaceDevice release its own registrations

[Césaire Mounah <cesaire.mounah@xxxxxxxxxx>]

The CDeviceList destructor called CleanupDeviceList(), which calls the
pure virtual OnDeviceRemoved() after the derived class has already been
destroyed.

The suspend registration and the shutdown and slate-mode watches are
registered on the device handle, but their contexts were kept in
CXenIfaceDeviceList, so only that callback could release them. Move the
contexts into CXenIfaceDevice and release them in its destructor, before
~CDevice() closes the handle. ~CDeviceList() now only deletes the
devices.

Assisted-by: Claude:Opus-5.5
Signed-off-by: Césaire Mounah <cesaire.mounah@xxxxxxxxxx>
---
 src/xenagent/devicelist.cpp     |   8 +-
 src/xenagent/xenifacedevice.cpp | 170 +++++++++++++++++---------------
 src/xenagent/xenifacedevice.h   |  20 ++--
 3 files changed, 108 insertions(+), 90 deletions(-)

diff --git a/src/xenagent/devicelist.cpp b/src/xenagent/devicelist.cpp
index 8bf9b2b..63aa7dd 100644
--- a/src/xenagent/devicelist.cpp
+++ b/src/xenagent/devicelist.cpp
@@ -185,7 +185,13 @@ CDeviceList::CDeviceList(const GUID& itf) :
 CDeviceList::~CDeviceList()
 {
     UnregisterForDeviceChange();
-    CleanupDeviceList();
+
+    for (DeviceMap::iterator it = m_devs.begin();
+         it != m_devs.end();
+         ++it) {
+        delete *it;
+    }
+    m_devs.clear();
 
     DeleteCriticalSection(&m_crit);
 }
diff --git a/src/xenagent/xenifacedevice.cpp b/src/xenagent/xenifacedevice.cpp
index 763abf0..d3e02a9 100644
--- a/src/xenagent/xenifacedevice.cpp
+++ b/src/xenagent/xenifacedevice.cpp
@@ -48,11 +48,18 @@
 
 #define TIME_SYNC_MODE_DISABLED 0
 
-CXenIfaceDevice::CXenIfaceDevice(const wchar_t* path) : CDevice(path)
+CXenIfaceDevice::CXenIfaceDevice(const wchar_t* path) : CDevice(path),
+    m_ctxt_suspend(NULL),
+    m_ctxt_shutdown(NULL),
+    m_ctxt_slate_mode(NULL)
 {}
 
 /*virtual*/ CXenIfaceDevice::~CXenIfaceDevice()
-{}
+{
+    StopSlateModeWatch();
+    StopShutdownWatch();
+    StopSuspendWatch();
+}
 
 // store interface
 bool CXenIfaceDevice::StoreRead(const std::string& path, std::string& value)
@@ -182,11 +189,72 @@ bool CXenIfaceDevice::Log(const std::string& msg)
                  NULL, 0);
 }
 
+void CXenIfaceDevice::StartSuspendWatch(HANDLE evt)
+{
+    if (m_ctxt_suspend)
+        return;
+
+    SuspendRegister(evt, &m_ctxt_suspend);
+}
+
+void CXenIfaceDevice::StopSuspendWatch()
+{
+    if (!m_ctxt_suspend)
+        return;
+
+    SuspendDeregister(m_ctxt_suspend);
+    m_ctxt_suspend = NULL;
+}
+
+void CXenIfaceDevice::StartShutdownWatch(HANDLE evt)
+{
+    if (m_ctxt_shutdown)
+        return;
+
+    StoreAddWatch("control/shutdown", evt, &m_ctxt_shutdown);
+
+    StoreWrite("control/feature-poweroff", "1");
+    StoreWrite("control/feature-reboot", "1");
+    StoreWrite("control/feature-s3", "1");
+    StoreWrite("control/feature-s4", "1");
+}
+
+void CXenIfaceDevice::StopShutdownWatch()
+{
+    if (!m_ctxt_shutdown)
+        return;
+
+    StoreWrite("control/feature-poweroff", "");
+    StoreWrite("control/feature-reboot", "");
+    StoreWrite("control/feature-s3", "");
+    StoreWrite("control/feature-s4", "");
+
+    StoreRemoveWatch(m_ctxt_shutdown);
+    m_ctxt_shutdown = NULL;
+}
+
+void CXenIfaceDevice::StartSlateModeWatch(HANDLE evt)
+{
+    if (m_ctxt_slate_mode)
+        return;
+
+    StoreAddWatch("control/laptop-slate-mode", evt, &m_ctxt_slate_mode);
+    StoreWrite("control/feature-laptop-slate-mode", "1");
+}
+
+void CXenIfaceDevice::StopSlateModeWatch()
+{
+    if (!m_ctxt_slate_mode)
+        return;
+
+    StoreRemove("control/feature-laptop-slate-mode");
+
+    StoreRemoveWatch(m_ctxt_slate_mode);
+    m_ctxt_slate_mode = NULL;
+}
+
 CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) : 
CDeviceList(GUID_INTERFACE_XENIFACE),
-    m_agent(agent),
-    m_ctxt_suspend(NULL),
-    m_ctxt_shutdown(NULL),
-    m_ctxt_slate_mode(NULL)
+    m_agent(agent)
 {
     m_evt_shutdown = CreateEvent(NULL, TRUE, FALSE, NULL);
     m_evt_suspend = CreateEvent(NULL, TRUE, FALSE, NULL);
@@ -215,11 +283,11 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* 
agent) : CDeviceList(GUID_IN
 
     CXenIfaceDevice* device = (CXenIfaceDevice*)dev;
 
-    device->SuspendRegister(m_evt_suspend, &m_ctxt_suspend);
-    StartShutdownWatch(device);
+    device->StartSuspendWatch(m_evt_suspend);
+    device->StartShutdownWatch(m_evt_shutdown);
 
     if (m_agent->ConvDevicePresent())
-        StartSlateModeWatch(device);
+        device->StartSlateModeWatch(m_evt_slate_mode);
 
     SetXenTime(device, false);
 }
@@ -229,17 +297,9 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) 
: CDeviceList(GUID_IN
     CCritSec crit(&m_crit);
     CXenIfaceDevice* device = (CXenIfaceDevice*)dev;
 
-    if (dev != GetFirstDevice())
-        return;
-
-    if (m_ctxt_suspend)
-        device->SuspendDeregister(m_ctxt_suspend);
-    m_ctxt_suspend = NULL;
-
-    if (m_agent->ConvDevicePresent())
-        StopSlateModeWatch(device);
-
-    StopShutdownWatch(device);
+    device->StopSuspendWatch();
+    device->StopSlateModeWatch();
+    device->StopShutdownWatch();
 }
 
 /*virtual*/ void CXenIfaceDeviceList::OnDeviceSuspend(CDevice* dev)
@@ -247,13 +307,8 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* agent) 
: CDeviceList(GUID_IN
     CCritSec crit(&m_crit);
     CXenIfaceDevice* device = (CXenIfaceDevice*)dev;
 
-    if (dev != GetFirstDevice())
-        return;
-
-    if (m_agent->ConvDevicePresent())
-        StopSlateModeWatch(device);
-
-    StopShutdownWatch(device);
+    device->StopSlateModeWatch();
+    device->StopShutdownWatch();
 }
 
 /*virtual*/ void CXenIfaceDeviceList::OnDeviceResume(CDevice* dev)
@@ -264,10 +319,10 @@ CXenIfaceDeviceList::CXenIfaceDeviceList(CXenAgent* 
agent) : CDeviceList(GUID_IN
     if (dev != GetFirstDevice())
         return;
 
-    StartShutdownWatch(device);
+    device->StartShutdownWatch(m_evt_shutdown);
 
     if (m_agent->ConvDevicePresent())
-        StartSlateModeWatch(device);
+        device->StartSlateModeWatch(m_evt_slate_mode);
 }
 
 void CXenIfaceDeviceList::Log(const char* message)
@@ -379,15 +434,13 @@ void CXenIfaceDeviceList::CheckSuspend()
     m_agent->EventLog(EVENT_XENUSER_UNSUSPENDED);
 
     // recreate watches, as suspending deactivated the watch
-    if (m_agent->ConvDevicePresent())
-        StopSlateModeWatch(device);
-
-    StopShutdownWatch(device);
+    device->StopSlateModeWatch();
+    device->StopShutdownWatch();
 
-    StartShutdownWatch(device);
+    device->StartShutdownWatch(m_evt_shutdown);
 
     if (m_agent->ConvDevicePresent())
-        StartSlateModeWatch(device);
+        device->StartSlateModeWatch(m_evt_slate_mode);
 
     m_count = count;
 }
@@ -427,53 +480,6 @@ void CXenIfaceDeviceList::LogIfRebootPending()
     CXenAgent::Log("RebootRequired detected\n");
 }
 
-void CXenIfaceDeviceList::StartShutdownWatch(CXenIfaceDevice* device)
-{
-    if (m_ctxt_shutdown)
-        return;
-
-    device->StoreAddWatch("control/shutdown", m_evt_shutdown, 
&m_ctxt_shutdown);
-
-    device->StoreWrite("control/feature-poweroff", "1");
-    device->StoreWrite("control/feature-reboot", "1");
-    device->StoreWrite("control/feature-s3", "1");
-    device->StoreWrite("control/feature-s4", "1");
-}
-
-void CXenIfaceDeviceList::StopShutdownWatch(CXenIfaceDevice* device)
-{
-    if (!m_ctxt_shutdown)
-        return;
-
-    device->StoreWrite("control/feature-poweroff", "");
-    device->StoreWrite("control/feature-reboot", "");
-    device->StoreWrite("control/feature-s3", "");
-    device->StoreWrite("control/feature-s4", "");
-
-    device->StoreRemoveWatch(m_ctxt_shutdown);
-    m_ctxt_shutdown = NULL;
-}
-
-void CXenIfaceDeviceList::StartSlateModeWatch(CXenIfaceDevice* device)
-{
-    if (m_ctxt_slate_mode)
-        return;
-
-    device->StoreAddWatch("control/laptop-slate-mode", m_evt_slate_mode, 
&m_ctxt_slate_mode);
-    device->StoreWrite("control/feature-laptop-slate-mode", "1");
-}
-
-void CXenIfaceDeviceList::StopSlateModeWatch(CXenIfaceDevice* device)
-{
-    if (!m_ctxt_slate_mode)
-        return;
-
-    device->StoreRemove("control/feature-laptop-slate-mode");
-
-    device->StoreRemoveWatch(m_ctxt_slate_mode);
-    m_ctxt_slate_mode = NULL;
-}
-
 void CXenIfaceDeviceList::AcquireShutdownPrivilege()
 {
     HANDLE          token;
diff --git a/src/xenagent/xenifacedevice.h b/src/xenagent/xenifacedevice.h
index 8a89036..1ddd454 100644
--- a/src/xenagent/xenifacedevice.h
+++ b/src/xenagent/xenifacedevice.h
@@ -60,6 +60,19 @@ public: // sharedinfo interface
 
 public: // logging
     bool Log(const std::string& msg);
+
+public:
+    void StartSuspendWatch(HANDLE evt);
+    void StopSuspendWatch();
+    void StartShutdownWatch(HANDLE evt);
+    void StopShutdownWatch();
+    void StartSlateModeWatch(HANDLE evt);
+    void StopSlateModeWatch();
+
+private:
+    void*   m_ctxt_suspend;
+    void*   m_ctxt_shutdown;
+    void*   m_ctxt_slate_mode;
 };
 
 class CXenAgent;
@@ -90,19 +103,12 @@ public:
     void LogIfRebootPending();
 
 private:
-    void StartShutdownWatch(CXenIfaceDevice* device);
-    void StopShutdownWatch(CXenIfaceDevice* device);
-    void StartSlateModeWatch(CXenIfaceDevice* device);
-    void StopSlateModeWatch(CXenIfaceDevice* device);
     void AcquireShutdownPrivilege();
     void SetXenTime(CXenIfaceDevice* device, bool forced);
 
 private:
     CXenAgent*  m_agent;
     DWORD       m_count;
-    void*       m_ctxt_suspend;
-    void*       m_ctxt_shutdown;
-    void*       m_ctxt_slate_mode;
 };
 
 #endif
-- 
2.55.0.windows.5



--
Césaire Mounah | Vates Windows Guest Tools Engineer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech