[PATCH 1/2] ntdll: Improve NtSetThreadExecutionState() stub.

[Chip Davis]

Signed-off-by: Chip Davis <cdavis(a)codeweavers.com>
---
 dlls/ntdll/nt.c         | 10 +++++-----
 dlls/ntdll/ntdll_misc.h |  1 +
 dlls/ntdll/server.c     |  1 +
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index a9985fbc224..c1b0816a35e 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
  */
 NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
 {
-    static EXECUTION_STATE current =
-        ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
-    *old_state = current;
+    struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
+
+    *old_state = thread_data->exec_state;
 
     WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
 
-    if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
-        current = new_state;
+    if (new_state & ES_CONTINUOUS)
+        thread_data->exec_state = new_state;
     return STATUS_SUCCESS;
 }
 
diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h
index 4556e1e5ce3..30080a47e53 100644
--- a/dlls/ntdll/ntdll_misc.h
+++ b/dlls/ntdll/ntdll_misc.h
@@ -245,6 +245,7 @@ struct ntdll_thread_data
     int                wait_fd[2];    /* fd for sleeping server requests */
     BOOL               wow64_redir;   /* Wow64 filesystem redirection flag */
     pthread_t          pthread_id;    /* pthread thread id */
+    EXECUTION_STATE    exec_state;    /* current thread power execution state */
 };
 
 C_ASSERT( sizeof(struct ntdll_thread_data) <= sizeof(((TEB *)0)->GdiTebBatch) );
diff --git a/dlls/ntdll/server.c b/dlls/ntdll/server.c
index 33ba95c00cc..385f5af6ebb 100644
--- a/dlls/ntdll/server.c
+++ b/dlls/ntdll/server.c
@@ -1525,6 +1525,7 @@ size_t server_init_thread( void *entry_point, BOOL *suspend )
 
     is_wow64 = !is_win64 && (server_cpus & ((1 << CPU_x86_64) | (1 << CPU_ARM64))) != 0;
     ntdll_get_thread_data()->wow64_redir = is_wow64;
+    ntdll_get_thread_data()->exec_state  = ES_CONTINUOUS;
 
     switch (ret)
     {
-- 
2.21.0

[PATCH 2/2] powrprof/tests: Test NtPowerInformation with SystemExecutionState.

[Chip Davis]

Test interaction between SetThreadExecutionState(), PowerSetRequest(),
and this parameter.

Signed-off-by: Chip Davis <cdavis(a)codeweavers.com>
---
 configure                       |   1 +
 configure.ac                    |   1 +
 dlls/powrprof/tests/Makefile.in |   4 +
 dlls/powrprof/tests/powrprof.c  | 223 ++++++++++++++++++++++++++++++++
 include/powrprof.h              |   5 +
 5 files changed, 234 insertions(+)
 create mode 100644 dlls/powrprof/tests/Makefile.in
 create mode 100644 dlls/powrprof/tests/powrprof.c

diff --git a/configure.ac b/configure.ac
index c69c5ecb45b..a3b0f1eac01 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3578,6 +3578,7 @@ WINE_CONFIG_MAKEFILE(dlls/pdh/tests)
 WINE_CONFIG_MAKEFILE(dlls/photometadatahandler)
 WINE_CONFIG_MAKEFILE(dlls/pidgen)
 WINE_CONFIG_MAKEFILE(dlls/powrprof)
+WINE_CONFIG_MAKEFILE(dlls/powrprof/tests)
 WINE_CONFIG_MAKEFILE(dlls/printui)
 WINE_CONFIG_MAKEFILE(dlls/prntvpt)
 WINE_CONFIG_MAKEFILE(dlls/propsys)
diff --git a/dlls/powrprof/tests/Makefile.in b/dlls/powrprof/tests/Makefile.in
new file mode 100644
index 00000000000..87aa06be502
--- /dev/null
+++ b/dlls/powrprof/tests/Makefile.in
@@ -0,0 +1,4 @@
+TESTDLL   = powrprof.dll
+IMPORTS   = powrprof
+
+C_SRCS    = powrprof.c
diff --git a/dlls/powrprof/tests/powrprof.c b/dlls/powrprof/tests/powrprof.c
new file mode 100644
index 00000000000..cd72ae2ff20
--- /dev/null
+++ b/dlls/powrprof/tests/powrprof.c
@@ -0,0 +1,223 @@
+/*
+ * Copyright 2019 Chip Davis for CodeWeavers
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ *
+ */
+
+#include <stdarg.h>
+#include <stdio.h>
+
+#include "ntstatus.h"
+#define WIN32_NO_STATUS
+#include "windef.h"
+#include "winbase.h"
+#include "powrprof.h"
+
+#include "wine/test.h"
+
+HANDLE (WINAPI *pPowerCreateRequest)(REASON_CONTEXT *);
+BOOL (WINAPI *pPowerSetRequest)(HANDLE, POWER_REQUEST_TYPE);
+BOOL (WINAPI *pPowerClearRequest)(HANDLE, POWER_REQUEST_TYPE);
+
+static void test_system_execution_state(void)
+{
+    EXECUTION_STATE es, old_es;
+    NTSTATUS status;
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+
+    old_es = SetThreadExecutionState(ES_SYSTEM_REQUIRED);
+    ok(old_es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", old_es);
+
+    old_es = es;
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(es == old_es, "unexpected execution state 0x%08x vs 0x%08x\n", es, old_es);
+
+    old_es = SetThreadExecutionState(ES_CONTINUOUS | ES_DISPLAY_REQUIRED);
+    ok(old_es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", old_es);
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    todo_wine ok(es & ES_DISPLAY_REQUIRED, "unexpected execution state 0x%08x\n", es);
+
+    old_es = SetThreadExecutionState(ES_CONTINUOUS);
+    ok(old_es == (ES_CONTINUOUS|ES_DISPLAY_REQUIRED), "unexpected execution state 0x%08x\n", old_es);
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+
+    old_es = SetThreadExecutionState(ES_DISPLAY_REQUIRED);
+    ok(old_es & ES_CONTINUOUS, "unexpected execution state 0x%08x\n", old_es);
+
+    old_es = es;
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(es == old_es, "unexpected execution state 0x%08x vs 0x%08x\n", es, old_es);
+}
+
+static HANDLE events[2];
+
+static DWORD CALLBACK execution_state_thread(LPVOID param)
+{
+    EXECUTION_STATE old_es;
+
+    SetThreadExecutionState(ES_SYSTEM_REQUIRED);
+    SetEvent(events[0]);
+    WaitForSingleObject(events[1], INFINITE);
+
+    old_es = SetThreadExecutionState(ES_DISPLAY_REQUIRED);
+    ok(old_es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", old_es);
+    SetEvent(events[0]);
+    WaitForSingleObject(events[1], INFINITE);
+
+    old_es = SetThreadExecutionState(ES_CONTINUOUS | ES_SYSTEM_REQUIRED);
+    ok(old_es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", old_es);
+    SetEvent(events[0]);
+    WaitForSingleObject(events[1], INFINITE);
+
+    old_es = SetThreadExecutionState(ES_CONTINUOUS | ES_DISPLAY_REQUIRED);
+    ok(old_es == (ES_CONTINUOUS|ES_SYSTEM_REQUIRED), "unexpected execution state 0x%08x\n", old_es);
+    SetEvent(events[0]);
+    WaitForSingleObject(events[1], INFINITE);
+
+    old_es = SetThreadExecutionState(ES_CONTINUOUS);
+    ok(old_es == (ES_CONTINUOUS|ES_DISPLAY_REQUIRED), "unexpected execution state 0x%08x\n", old_es);
+
+    return 0;
+}
+
+static void test_system_execution_state_other_thread(void)
+{
+    HANDLE thread;
+    EXECUTION_STATE base_es, es;
+    NTSTATUS status;
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &base_es, sizeof(base_es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    trace("base execution state = 0x%08x\n", base_es);
+
+    events[0] = CreateEventW(NULL, FALSE, FALSE, NULL);
+    events[1] = CreateEventW(NULL, FALSE, FALSE, NULL);
+    thread = CreateThread(NULL, 0, execution_state_thread, NULL, 0, NULL);
+    ok(thread != NULL, "Failed to create thread, err %u\n", GetLastError());
+
+    WaitForSingleObject(events[0], INFINITE);
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(es == base_es, "unexpected execution state 0x%08x\n", es);
+    es = SetThreadExecutionState(0);
+    ok(es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", es);
+    SetEvent(events[1]);
+
+    WaitForSingleObject(events[0], INFINITE);
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(es == base_es, "unexpected execution state 0x%08x\n", es);
+    es = SetThreadExecutionState(0);
+    ok(es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", es);
+    SetEvent(events[1]);
+
+    WaitForSingleObject(events[0], INFINITE);
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    todo_wine ok(es & ES_SYSTEM_REQUIRED, "unexpected execution state 0x%08x\n", es);
+    es = SetThreadExecutionState(0);
+    ok(es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", es);
+    SetEvent(events[1]);
+
+    WaitForSingleObject(events[0], INFINITE);
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    todo_wine ok(es & ES_DISPLAY_REQUIRED, "unexpected execution state 0x%08x\n", es);
+    es = SetThreadExecutionState(0);
+    ok(es == ES_CONTINUOUS, "unexpected execution state 0x%08x\n", es);
+    SetEvent(events[1]);
+
+    WaitForSingleObject(thread, INFINITE);
+    CloseHandle(thread);
+    CloseHandle(events[0]);
+    CloseHandle(events[1]);
+}
+
+static void test_system_execution_state_power_request()
+{
+    HANDLE req;
+    REASON_CONTEXT reason;
+    BOOL ret;
+    NTSTATUS status;
+    EXECUTION_STATE base_es, es;
+    static const WCHAR reasonW[] = {'W', 'i', 'n', 'e', ' ', 't', 'e', 's', 't', 0};
+
+    if (!pPowerCreateRequest)
+    {
+        win_skip("Power request objects unavailable\n");
+        return;
+    }
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &base_es, sizeof(base_es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    trace("base execution state = 0x%08x\n", base_es);
+
+    reason.Version = 0;
+    reason.Flags = POWER_REQUEST_CONTEXT_SIMPLE_STRING;
+    reason.Reason.SimpleReasonString = reasonW;
+    req = pPowerCreateRequest(&reason);
+    todo_wine ok(req != INVALID_HANDLE_VALUE, "err %u\n", GetLastError());
+
+    ret = pPowerSetRequest(req, PowerRequestSystemRequired);
+    todo_wine ok(ret, "err %u\n", GetLastError());
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    todo_wine ok(es & ES_SYSTEM_REQUIRED, "unexpected execution state 0x%08x\n", es);
+
+    ret = pPowerClearRequest(req, PowerRequestSystemRequired);
+    todo_wine ok(ret, "err %u\n", GetLastError());
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(!(es & ES_SYSTEM_REQUIRED) || (base_es & ES_SYSTEM_REQUIRED), "unexpected execution state 0x%08x\n", es);
+
+    ret = pPowerSetRequest(req, PowerRequestDisplayRequired);
+    todo_wine ok(ret, "err %u\n", GetLastError());
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    todo_wine ok(es & ES_DISPLAY_REQUIRED, "unexpected execution state 0x%08x\n", es);
+
+    ret = CloseHandle(req);
+    todo_wine ok(ret, "err %u\n", GetLastError());
+
+    status = CallNtPowerInformation(SystemExecutionState, NULL, 0, &es, sizeof(es));
+    ok(status == STATUS_SUCCESS, "status %08x\n", status);
+    ok(!(es & ES_DISPLAY_REQUIRED) || (base_es & ES_DISPLAY_REQUIRED), "unexpected execution state 0x%08x\n", es);
+}
+
+START_TEST(powrprof)
+{
+    HMODULE hkernel32;
+
+    hkernel32 = GetModuleHandleA("kernel32.dll");
+    pPowerCreateRequest = (void *)GetProcAddress(hkernel32, "PowerCreateRequest");
+    pPowerSetRequest = (void *)GetProcAddress(hkernel32, "PowerSetRequest");
+    pPowerClearRequest = (void *)GetProcAddress(hkernel32, "PowerClearRequest");
+
+    test_system_execution_state();
+    test_system_execution_state_other_thread();
+    test_system_execution_state_power_request();
+}
diff --git a/include/powrprof.h b/include/powrprof.h
index 318188e465e..421d9d4fc76 100644
--- a/include/powrprof.h
+++ b/include/powrprof.h
@@ -146,6 +146,11 @@ extern "C" {
 
 typedef BOOLEAN (CALLBACK* PWRSCHEMESENUMPROC)(UINT, DWORD, LPWSTR, DWORD, LPWSTR, PPOWER_POLICY, LPARAM);
 
+#ifndef WINE_NTSTATUS_DECLARED
+#define WINE_NTSTATUS_DECLARED
+typedef LONG NTSTATUS;
+#endif
+
 NTSTATUS WINAPI CallNtPowerInformation(POWER_INFORMATION_LEVEL, PVOID, ULONG, PVOID, ULONG);
 BOOLEAN WINAPI CanUserWritePwrScheme(VOID);
 BOOLEAN WINAPI DeletePwrScheme(UINT);
-- 
2.21.0

[PATCH 2/2] powrprof/tests: Test NtPowerInformation with SystemExecutionState.

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=55258

Your paranoid android.


=== wvistau64 (64 bit report) ===

powrprof:
powrprof: Timeout

=== w2008s64 (64 bit report) ===

powrprof:
powrprof: Timeout

=== w7pro64 (task log) ===

Task errors:
The task timed out

=== w864 (64 bit report) ===

powrprof:
powrprof: Timeout

=== w1064v1507 (64 bit report) ===

powrprof:
powrprof: Timeout

=== w1064v1809 (64 bit report) ===

powrprof:
powrprof: Timeout

[PATCH 2/2] powrprof/tests: Test NtPowerInformation with SystemExecutionState.

[Chip Davis]

August 7, 2019 2:49 PM, "Marvin" <testbot(a)winehq.org> wrote:
> === wvistau64 (64 bit report) ===
>
> powrprof:
> powrprof: Timeout
>
> === w2008s64 (64 bit report) ===
>
> powrprof:
> powrprof: Timeout
>
> === w7pro64 (task log) ===
>
> Task errors:
> The task timed out
>
> === w864 (64 bit report) ===
>
> powrprof:
> powrprof: Timeout
>
> === w1064v1507 (64 bit report) ===
>
> powrprof:
> powrprof: Timeout
>
> === w1064v1809 (64 bit report) ===
>
> powrprof:
> powrprof: Timeout

I have no idea why the 64-bit tasks (and only the 64-bit tasks) time out. AFAICT, it isn't even reaching the test handler.

Chip

[PATCH 2/2] powrprof/tests: Test NtPowerInformation with SystemExecutionState.

[Paul Gofman]

My user32 tests for 64 bit fail the same way. It looks like some
regression in 64-bit tests in the latest git. 64-bit tests show message
"The procedure entry point __p__job could not be located in the dynamic
link library msvcrt.dll." both in final testbot screenshots and my
locally built 64-bit tests on Windows 7 machine.

On 8/7/19 22:52, Chip Davis wrote:
> August 7, 2019 2:49 PM, "Marvin" <testbot(a)winehq.org> wrote:
> > === wvistau64 (64 bit report) ===
> >
> > powrprof:
> > powrprof: Timeout
> >
> > === w2008s64 (64 bit report) ===
> >
> > powrprof:
> > powrprof: Timeout
> >
> > === w7pro64 (task log) ===
> >
> > Task errors:
> > The task timed out
> >
> > === w864 (64 bit report) ===
> >
> > powrprof:
> > powrprof: Timeout
> >
> > === w1064v1507 (64 bit report) ===
> >
> > powrprof:
> > powrprof: Timeout
> >
> > === w1064v1809 (64 bit report) ===
> >
> > powrprof:
> > powrprof: Timeout
> > I have no idea why the 64-bit tasks (and only the 64-bit tasks) time out. AFAICT, it isn't even reaching the test handler.
>
> Chip

[Alexandre Julliard]

Chip Davis <cdavis(a)codeweavers.com> writes:
> @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
>   */
>  NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
>  {
> -    static EXECUTION_STATE current =
> -        ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> -    *old_state = current;
> +    struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> +
> +    *old_state = thread_data->exec_state;
>
> WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
>
> -    if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> -        current = new_state;
> +    if (new_state & ES_CONTINUOUS)
> +        thread_data->exec_state = new_state;
>      return STATUS_SUCCESS;
>  }

This would most likely need to be stored in the server, it doesn't seem
very useful to have in the local thread data.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Chip Davis]

August 13, 2019 4:10 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> Chip Davis <cdavis(a)codeweavers.com> writes:
> > @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
> > */
> > NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
> > {
> > - static EXECUTION_STATE current =
> > - ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> > - *old_state = current;
> > + struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> > +
> > + *old_state = thread_data->exec_state;
> >
> > WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
> >
> > - if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> > - current = new_state;
> > + if (new_state & ES_CONTINUOUS)
> > + thread_data->exec_state = new_state;
> > return STATUS_SUCCESS;
> > }
>
> This would most likely need to be stored in the server, it doesn't seem
> very useful to have in the local thread data.

My tests (patch 2) demonstrate that there are two distinct states here: the thread execution state, which is unique to each thread; and the system execution state, which is the union of all threads' execution states and active power request objects. Note in particular that setting thread A's execution state does not affect the return value of SetThreadExecutionState() for thread B.
> --
> Alexandre Julliard
> julliard(a)winehq.org

Chip

[Alexandre Julliard]

"Chip Davis" <cdavis(a)codeweavers.com> writes:
> August 13, 2019 4:10 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> > Chip Davis <cdavis(a)codeweavers.com> writes:
> > > @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
> > > */
> > > NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
> > > {
> > > - static EXECUTION_STATE current =
> > > - ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> > > - *old_state = current;
> > > + struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> > > +
> > > + *old_state = thread_data->exec_state;
> > >
> > > WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
> > >
> > > - if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> > > - current = new_state;
> > > + if (new_state & ES_CONTINUOUS)
> > > + thread_data->exec_state = new_state;
> > > return STATUS_SUCCESS;
> > > }
> >
> > This would most likely need to be stored in the server, it doesn't seem
> > very useful to have in the local thread data.
>
> My tests (patch 2) demonstrate that there are two distinct states
> here: the thread execution state, which is unique to each thread; and
> the system execution state, which is the union of all threads'
> execution states and active power request objects. Note in particular
> that setting thread A's execution state does not affect the return
> value of SetThreadExecutionState() for thread B.

Yes of course, but you won't be able to compute the system state if you
store the thread state in the thread local data.

-- 
Alexandre Julliard
julliard(a)winehq.org

[Chip Davis]

August 13, 2019 12:31 PM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> "Chip Davis" <cdavis(a)codeweavers.com> writes:
> > August 13, 2019 4:10 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> > > Chip Davis <cdavis(a)codeweavers.com> writes:
> >
> > @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
> > */
> > NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
> > {
> > - static EXECUTION_STATE current =
> > - ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> > - *old_state = current;
> > + struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> > +
> > + *old_state = thread_data->exec_state;
> >
> > WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
> >
> > - if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> > - current = new_state;
> > + if (new_state & ES_CONTINUOUS)
> > + thread_data->exec_state = new_state;
> > return STATUS_SUCCESS;
> > }
> > > This would most likely need to be stored in the server, it doesn't seem
> > > very useful to have in the local thread data.
> >
> > My tests (patch 2) demonstrate that there are two distinct states
> > here: the thread execution state, which is unique to each thread; and
> > the system execution state, which is the union of all threads'
> > execution states and active power request objects. Note in particular
> > that setting thread A's execution state does not affect the return
> > value of SetThreadExecutionState() for thread B.
>
> Yes of course, but you won't be able to compute the system state if you
> store the thread state in the thread local data.

See patch 3 in the new series I just sent.

Chip

[Chip Davis]

August 13, 2019 12:34 PM, "Chip Davis" <cdavis(a)codeweavers.com> wrote:
> August 13, 2019 12:31 PM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> > "Chip Davis" <cdavis(a)codeweavers.com> writes:
> > > August 13, 2019 4:10 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> >
> > Chip Davis <cdavis(a)codeweavers.com> writes:
> > > @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
> > > */
> > > NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
> > > {
> > > - static EXECUTION_STATE current =
> > > - ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> > > - *old_state = current;
> > > + struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> > > +
> > > + *old_state = thread_data->exec_state;
> > >
> > > WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
> > >
> > > - if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> > > - current = new_state;
> > > + if (new_state & ES_CONTINUOUS)
> > > + thread_data->exec_state = new_state;
> > > return STATUS_SUCCESS;
> > > }
> >
> > This would most likely need to be stored in the server, it doesn't seem
> > very useful to have in the local thread data.
> > > My tests (patch 2) demonstrate that there are two distinct states
> > > here: the thread execution state, which is unique to each thread; and
> > > the system execution state, which is the union of all threads'
> > > execution states and active power request objects. Note in particular
> > > that setting thread A's execution state does not affect the return
> > > value of SetThreadExecutionState() for thread B.
> >
> > Yes of course, but you won't be able to compute the system state if you
> > store the thread state in the thread local data.
>
> See patch 3 in the new series I just sent.

...And it just occurred to me that this won't work correctly when the thread terminates. Disregard.

[Alexandre Julliard]

"Chip Davis" <cdavis(a)codeweavers.com> writes:
> August 13, 2019 12:31 PM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> > "Chip Davis" <cdavis(a)codeweavers.com> writes:
> > > August 13, 2019 4:10 AM, "Alexandre Julliard" <julliard(a)winehq.org> wrote:
> > > > Chip Davis <cdavis(a)codeweavers.com> writes:
> > >
> > > @@ -3062,14 +3062,14 @@ NTSTATUS WINAPI NtInitiatePowerAction(
> > > */
> > > NTSTATUS WINAPI NtSetThreadExecutionState( EXECUTION_STATE new_state, EXECUTION_STATE *old_state )
> > > {
> > > - static EXECUTION_STATE current =
> > > - ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED | ES_USER_PRESENT;
> > > - *old_state = current;
> > > + struct ntdll_thread_data *thread_data = ntdll_get_thread_data();
> > > +
> > > + *old_state = thread_data->exec_state;
> > >
> > > WARN( "(0x%x, %p): stub, harmless.\n", new_state, old_state );
> > >
> > > - if (!(current & ES_CONTINUOUS) || (new_state & ES_CONTINUOUS))
> > > - current = new_state;
> > > + if (new_state & ES_CONTINUOUS)
> > > + thread_data->exec_state = new_state;
> > > return STATUS_SUCCESS;
> > > }
> > > > This would most likely need to be stored in the server, it doesn't seem
> > > > very useful to have in the local thread data.
> > >
> > > My tests (patch 2) demonstrate that there are two distinct states
> > > here: the thread execution state, which is unique to each thread; and
> > > the system execution state, which is the union of all threads'
> > > execution states and active power request objects. Note in particular
> > > that setting thread A's execution state does not affect the return
> > > value of SetThreadExecutionState() for thread B.
> >
> > Yes of course, but you won't be able to compute the system state if you
> > store the thread state in the thread local data.
>
> See patch 3 in the new series I just sent.

You really need to store it per-thread on the server side. Otherwise you
can't clean up at thread exit etc.  You could possibly also store it as
a power request to simplify the code.

-- 
Alexandre Julliard
julliard(a)winehq.org