[Spice-devel] [vdagent-win v3 2/2] vdagent-win: start vdagent with lock info from session
Frediano Ziglio
fziglio at redhat.com
Fri Aug 12 13:52:55 UTC 2016
>
> 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.
>
> As the VDService should always set the unlock event in case client
> connects to a unlocked Session, VDAgent now consider the session
> locked till Session or VDService says its not.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1323628
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> Reported-by: Andrei Stepanov <astepano at redhat.com>
> ---
>
> Changed the approach to be in the safe side. VDAgent starts now with the
> session_is_locked property set to true and that should only change by VDAgent
> _session_unlocked_event event (on startup) or by Session WTS_SESSION_UNLOCK
> event.
>
> ---
> common/vdcommon.h | 1 +
> vdagent/vdagent.cpp | 18 ++++++++++++++++--
> vdservice/vdservice.cpp | 15 +++++++++++++++
> 3 files changed, 32 insertions(+), 2 deletions(-)
>
> 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..aa64824 100644
> --- a/vdagent/vdagent.cpp
> +++ b/vdagent/vdagent.cpp
> @@ -143,6 +143,7 @@ private:
> DWORD _input_time;
> HANDLE _control_event;
> HANDLE _stop_event;
> + HANDLE _session_unlocked_event;
> VDAgentMessage* _in_msg;
> uint32_t _in_msg_pos;
> bool _pending_input;
> @@ -203,11 +204,12 @@ VDAgent::VDAgent()
> , _input_time (0)
> , _control_event (NULL)
> , _stop_event (NULL)
> + , _session_unlocked_event (NULL)
> , _in_msg (NULL)
> , _in_msg_pos (0)
> , _pending_input (false)
> , _running (false)
> - , _session_is_locked (false)
> + , _session_is_locked (true)
> , _desktop_switch (false)
> , _desktop_layout (NULL)
> , _display_setting (VD_AGENT_REGISTRY_KEY)
> @@ -309,6 +311,7 @@ bool VDAgent::run()
> return false;
> }
> _stop_event = OpenEvent(SYNCHRONIZE, FALSE, VD_AGENT_STOP_EVENT);
> + _session_unlocked_event = OpenEvent(SYNCHRONIZE, FALSE,
> VD_AGENT_SESSION_UNLOCKED_EVENT);
> memset(&wcls, 0, sizeof(wcls));
> wcls.lpfnWndProc = &VDAgent::wnd_proc;
> wcls.lpszClassName = VD_AGENT_WINCLASS_NAME;
> @@ -481,13 +484,14 @@ void VDAgent::input_desktop_message_loop()
>
> void VDAgent::event_dispatcher(DWORD timeout, DWORD wake_mask)
> {
> - HANDLE events[2];
> + HANDLE events[3];
> DWORD event_count = 1;
> DWORD wait_ret;
> MSG msg;
> enum {
> CONTROL_ACTION,
> STOP_ACTION,
> + SESSION_UNLOCKED_ACTION,
> } actions[SPICE_N_ELEMENTS(events)], action;
>
> events[0] = _control_event;
> @@ -498,6 +502,12 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD
> wake_mask)
> event_count++;
> }
>
> + if (_session_unlocked_event) {
> + events[event_count] = _session_unlocked_event;
> + actions[event_count] = SESSION_UNLOCKED_ACTION;
> + event_count++;
> + }
> +
> wait_ret = MsgWaitForMultipleObjectsEx(event_count, events, timeout,
> wake_mask, MWMO_ALERTABLE);
> if (wait_ret == WAIT_OBJECT_0 + event_count) {
> while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) {
> @@ -522,6 +532,10 @@ void VDAgent::event_dispatcher(DWORD timeout, DWORD
> wake_mask)
> vd_printf("%s: received stop event", __func__);
> _running = false;
> break;
> + case SESSION_UNLOCKED_ACTION:
> + vd_printf("%s: received session unlocked event", __func__);
> + _session_is_locked = false;
> + break;
> default:
> vd_printf("%s: action not handled (%d)", __func__, action);
> _running = false;
> diff --git a/vdservice/vdservice.cpp b/vdservice/vdservice.cpp
> index 89e0dbb..0df0c52 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;
> }
>
Mumble... got an idea about this. Why instead of using the event as
an event you use it as a flag? So on the service you set if unlocked
and reset if locked (the event must be on manual).
On the agent you use WaitForSingleObject with a 0 timeout just to
read the status, WAIT_OBJECT_0 if set or WAIT_TIMEOUT if unset.
Obviously you remove the event from the loop or if set you'll get
an high CPU usage.
I don't know if you can use WTSQuerySessionInformation to just
retrieve the state (lock/unlock) of the session.
Frediano
More information about the Spice-devel
mailing list