[Spice-devel] [vdagent-win v3 2/2] vdagent-win: start vdagent with lock info from session

[Victor Toso]

Hey,

On Fri, Aug 12, 2016 at 11:01:36AM -0400, Frediano Ziglio wrote:
> Would be something like this (not tested)
> 
> 
> diff --git a/common/vdcommon.h b/common/vdcommon.h
> index f4859e2..67fb034 100644
> --- a/common/vdcommon.h
> +++ b/common/vdcommon.h
> @@ -35,6 +35,7 @@ typedef CRITICAL_SECTION mutex_t;
>  
>  #define VD_AGENT_REGISTRY_KEY "SOFTWARE\\Red Hat\\Spice\\vdagent\\"
>  #define VD_AGENT_STOP_EVENT   TEXT("Global\\vdagent_stop_event")
> +#define VD_AGENT_SESSION_UNLOCKED_EVENT TEXT("Global\\vdagent_session_unlocked_event")
>  
>  #if defined __GNUC__
>  #define ALIGN_GCC __attribute__ ((packed))
> diff --git a/vdagent/vdagent.cpp b/vdagent/vdagent.cpp
> index 8e7ba2b..0c24a92 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -236,6 +236,18 @@ VDAgent::VDAgent()
>      MUTEX_INIT(_control_mutex);
>      MUTEX_INIT(_message_mutex);
>  
> +    HANDLE _session_unlocked_event = OpenEvent(SYNCHRONIZE, FALSE, VD_AGENT_SESSION_UNLOCKED_EVENT);
> +    switch (WaitForSingleObject(_session_unlocked_event, 0)) {
> +    case WAIT_OBJECT_0:
> +        _session_is_locked = false;
> +        break;
> +    default:
> +    case WAIT_TIMEOUT:
> +        _session_is_locked = true;
> +        break;
> +    }
> +    CloseHandle(_session_unlocked_event);
> +

Okay, we should send the 'unlock' event as soon as possible then.

>      _singleton = this;
>  }
>  
> diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
> index 12f7644..e1476a0 100644
> --- a/vdservice/vdservice.cpp
> +++ b/vdservice/vdservice.cpp
> @@ -93,6 +93,7 @@ private:
>      PROCESS_INFORMATION _agent_proc_info;
>      HANDLE _control_event;
>      HANDLE _agent_stop_event;
> +    HANDLE _session_unlocked_event;
>      HANDLE* _events;
>      TCHAR _agent_path[MAX_PATH];
>      VDControlQueue _control_queue;
> @@ -135,6 +136,7 @@ VDService::VDService()
>      _system_version = supported_system_version();
>      _control_event = CreateEvent(NULL, FALSE, FALSE, NULL);
>      _agent_stop_event = CreateEvent(NULL, FALSE, FALSE, VD_AGENT_STOP_EVENT);
> +    _session_unlocked_event = CreateEvent(NULL, TRUE, TRUE, VD_AGENT_SESSION_UNLOCKED_EVENT);
>      _agent_path[0] = wchar_t('\0');
>      MUTEX_INIT(_agent_mutex);
>      MUTEX_INIT(_control_mutex);
> @@ -143,6 +145,7 @@ VDService::VDService()
>  
>  VDService::~VDService()
>  {
> +    CloseHandle(_session_unlocked_event);
>      CloseHandle(_agent_stop_event);
>      CloseHandle(_control_event);
>      delete[] _events;
> @@ -307,6 +310,10 @@ DWORD WINAPI VDService::control_handler(DWORD control, DWORD event_type, LPVOID
>          if (event_type == WTS_CONSOLE_CONNECT) {
>              s->_session_id = session_id;
>              s->set_control_event(VD_CONTROL_RESTART_AGENT);
> +        } else if (event_type == WTS_SESSION_LOCK) {
> +            ResetEvent(s->_session_unlocked_event);
> +        } else if (event_type == WTS_SESSION_UNLOCK) {
> +            SetEvent(s->_session_unlocked_event);

But here you only send it on control_handler, which means the VDAgent
will be on locked session by default at start.

I really think we need to save the lock state in VDService. With the
following change in your patch, it worked.

diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
index f413f59..7f98121 100644
--- a/vdservice/vdservice.cpp
+++ b/vdservice/vdservice.cpp
@@ -104,6 +104,7 @@ private:
     int _system_version;
     bool _agent_alive;
     bool _running;
+    bool _session_is_locked;
     VDLog* _log;
     unsigned _events_count;
 };
@@ -117,6 +118,7 @@ VDService::VDService()
     , _agent_restarts (0)
     , _agent_alive (false)
     , _running (false)
+    , _session_is_locked (false)
     , _log (NULL)
     , _events_count(0)
 {
@@ -298,9 +300,9 @@ DWORD WINAPI VDService::control_handler(DWORD control, DWORD event_type, LPVOID
             s->_session_id = session_id;
             s->set_control_event(VD_CONTROL_RESTART_AGENT);
         } else if (event_type == WTS_SESSION_LOCK) {
-            ResetEvent(s->_session_unlocked_event);
+            s->_session_is_locked = true;
         } else if (event_type == WTS_SESSION_UNLOCK) {
-            SetEvent(s->_session_unlocked_event);
+            s->_session_is_locked = false;
         }
         break;
     }
@@ -738,6 +740,11 @@ bool VDService::launch_agent()
         vd_printf("CreateProcess() failed: %lu", GetLastError());
         return false;
     }
+    if (_session_is_locked) {
+        ResetEvent(_session_unlocked_event);
+    } else {
+        SetEvent(_session_unlocked_event);
+    }
     _agent_alive = true;
     return true;
 }

Let me know if you agree with that.
  toso