[Spice-devel] [vdagent-win v5] vdagent-win: start vdagent with lock info from session

[Frediano Ziglio]

> 
> Commit 5907b6cbb5c724f9729da59a644271b4258d122e started to handle
> Lock/Unlock events from Session at VDAgent. Although that works just
> fine, it does not cover all the situations as pointed by Andrei at [0]
> and I quote:
> 
> > It fails for next test-case:
> >
> > * Connect with RV to VM
> > * Lock VM (ctrl-alt-del)
> > * Close RV
> > * Connect with RV to the VM again
> > * Do not unlock session. DnD files from client to locked VM.
> >
> > Result : Files are copied to VM.
> 
> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1323628#c6
> 
> To solve this situation, we need VDService to inform VDAgent if
> Session is Locked or not upon its initialization.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323628
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> Reported-by: Andrei Stepanov <astepano at redhat.com>
> ---
> Change from v4: Fixing commit log
> ---
>  common/vdcommon.h       |  1 +
>  vdagent/vdagent.cpp     | 14 ++++++++++++++
>  vdservice/vdservice.cpp | 15 +++++++++++++++
>  3 files changed, 30 insertions(+)
> 
> 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 2b63267..8523564 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -236,6 +236,20 @@ 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:
> +        vd_printf("Session is not locked (by event)");
> +        _session_is_locked = false;
> +        break;
> +    default:
> +    case WAIT_TIMEOUT:
> +        vd_printf("Session is locked");
> +        _session_is_locked = true;
> +        break;
> +    }
> +    CloseHandle(_session_unlocked_event);
> +
>      _singleton = this;
>  }
>  
> diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
> index 12f7644..a5784ea 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;
> @@ -105,6 +106,7 @@ private:
>      int _system_version;
>      bool _agent_alive;
>      bool _running;
> +    bool _session_is_locked;
>      VDLog* _log;
>      unsigned _events_count;
>  };
> @@ -128,6 +130,7 @@ VDService::VDService()
>      , _agent_restarts (0)
>      , _agent_alive (false)
>      , _running (false)
> +    , _session_is_locked (false)
>      , _log (NULL)
>      , _events_count(0)
>  {
> @@ -135,6 +138,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, FALSE, FALSE,
> VD_AGENT_SESSION_UNLOCKED_EVENT);
>      _agent_path[0] = wchar_t('\0');
>      MUTEX_INIT(_agent_mutex);
>      MUTEX_INIT(_control_mutex);
> @@ -143,6 +147,7 @@ VDService::VDService()
>  
>  VDService::~VDService()
>  {
> +    CloseHandle(_session_unlocked_event);
>      CloseHandle(_agent_stop_event);
>      CloseHandle(_control_event);
>      delete[] _events;
> @@ -307,6 +312,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) {
> +            s->_session_is_locked = true;
> +        } else if (event_type == WTS_SESSION_UNLOCK) {
> +            s->_session_is_locked = false;
>          }
>          break;
>      }
> @@ -744,6 +753,12 @@ bool VDService::launch_agent()
>          return false;
>      }
>      _agent_alive = true;
> +
> +    if (!_session_is_locked) {
> +        /* If we just create a new VDAgent but the session is locked, we
> must
> +         * notify it so it can handle requests from client correctly */
> +        SetEvent(_session_unlocked_event);
> +    }
>      return true;
>  }
>  

On the agent you are using my code but on the service you revert to
your code. Was it not working? I think combining automatic events and
setting after launching the process is more racy then before.

Frediano