Skip to content

Restore Win7 compat by using old versions of threadpool functions #895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kennykerr merged 1 commit into from
Mar 22, 2021
Merged

Restore Win7 compat by using old versions of threadpool functions #895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions strings/base_coroutine_threadpool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ WINRT_EXPORT namespace winrt
m_handle = handle;
m_timer.attach(check_pointer(WINRT_IMPL_CreateThreadpoolTimer(callback, this, nullptr)));
int64_t relative_count = -m_duration.count();
WINRT_IMPL_SetThreadpoolTimerEx(m_timer.get(), &relative_count, 0, 0);
WINRT_IMPL_SetThreadpoolTimer(m_timer.get(), &relative_count, 0, 0);

state expected = state::idle;
if (!m_state.compare_exchange_strong(expected, state::pending, std::memory_order_release))
Expand All @@ -435,10 +435,10 @@ WINRT_EXPORT namespace winrt

void fire_immediately() noexcept
{
if (WINRT_IMPL_SetThreadpoolTimerEx(m_timer.get(), nullptr, 0, 0))
if (WINRT_IMPL_SetThreadpoolTimer(m_timer.get(), nullptr, 0, 0))
Copy link

@ysc3839 ysc3839 Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to SetThreadpoolTimer docs, it returns void instead of BOOL, is it OK to use this return value?

Copy link
Collaborator

@kennykerr kennykerr Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted - @oldnewthing I'm guessing this means we don't have test coverage for this one. 😟

Copy link
Member Author

@oldnewthing oldnewthing Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, because there's a cancellation test. Will have to figure out how this slipped through. Probably because I messed up.

Copy link
Member Author

@oldnewthing oldnewthing Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, the deal is that this works on Win8+ due to how SetThreadpoolTimer is implemented (it calls SetThreadpoolTimerEx and throws away the result, but the result is still lying around in eax so we can still act on it). But this won't work on Win7, where eax does not contain anything meaningful. It looks like timer cancellation propagation will not work on Win7. Should I do GetProcAddress/dynamic light-up?

Copy link
Collaborator

@kennykerr kennykerr Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we'll have to do that. You can (almost) use load_runtime_function except that its currently only used for combase functions and the thread pool is implemented by kernel32.

cppwinrt/strings/base_agile_ref.h

Lines 124 to 140 in 1502e29

template <typename F, typename L>
void load_runtime_function(char const* name, F& result, L fallback) noexcept
{
if (result)
{
return;
}
result = reinterpret_cast<F>(WINRT_IMPL_GetProcAddress(WINRT_IMPL_LoadLibraryW(L"combase.dll"), name));
if (result)
{
return;
}
result = fallback;
}

This is the way Win7 fallback is handled, for example:

cppwinrt/strings/base_activation.h

Lines 21 to 38 in 1502e29

inline int32_t __stdcall fallback_RoGetActivationFactory(void*, guid const&, void** factory) noexcept
{
*factory = nullptr;
return error_class_not_available;
}
template <bool isSameInterfaceAsIActivationFactory>
WINRT_IMPL_NOINLINE hresult get_runtime_activation_factory_impl(param::hstring const& name, winrt::guid const& guid, void** result) noexcept
{
if (winrt_activation_handler)
{
return winrt_activation_handler(*(void**)(&name), guid, result);
}
static int32_t(__stdcall * handler)(void* classId, winrt::guid const& iid, void** factory) noexcept;
impl::load_runtime_function("RoGetActivationFactory", handler, fallback_RoGetActivationFactory);
hresult hr = handler(*(void**)(&name), guid, result);

Copy link
Member Author

@oldnewthing oldnewthing Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't quite do it that way because I want to disable a feature if the GetProcAddress fails. There is no "fallback" available. (I guess I could see if result == fallback_Blah.)

Copy link
Collaborator

@kennykerr kennykerr Mar 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever you think is best.

Copy link
Member Author

@oldnewthing oldnewthing Mar 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#898

{
int64_t now = 0;
WINRT_IMPL_SetThreadpoolTimerEx(m_timer.get(), &now, 0, 0);
WINRT_IMPL_SetThreadpoolTimer(m_timer.get(), &now, 0, 0);
}
}

Expand Down Expand Up @@ -513,7 +513,7 @@ WINRT_EXPORT namespace winrt
m_wait.attach(check_pointer(WINRT_IMPL_CreateThreadpoolWait(callback, this, nullptr)));
int64_t relative_count = -m_timeout.count();
int64_t* file_time = relative_count != 0 ? &relative_count : nullptr;
WINRT_IMPL_SetThreadpoolWaitEx(m_wait.get(), m_handle, file_time, nullptr);
WINRT_IMPL_SetThreadpoolWait(m_wait.get(), m_handle, file_time);

state expected = state::idle;
if (!m_state.compare_exchange_strong(expected, state::pending, std::memory_order_release))
Expand All @@ -535,10 +535,10 @@ WINRT_EXPORT namespace winrt

void fire_immediately() noexcept
{
if (WINRT_IMPL_SetThreadpoolWaitEx(m_wait.get(), nullptr, nullptr, nullptr))
if (WINRT_IMPL_SetThreadpoolWait(m_wait.get(), nullptr, nullptr))
{
int64_t now = 0;
WINRT_IMPL_SetThreadpoolWaitEx(m_wait.get(), WINRT_IMPL_GetCurrentProcess(), &now, nullptr);
WINRT_IMPL_SetThreadpoolWait(m_wait.get(), WINRT_IMPL_GetCurrentProcess(), &now);
}
}

Expand Down
8 changes: 4 additions & 4 deletions strings/base_extern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ extern "C"

int32_t __stdcall WINRT_IMPL_TrySubmitThreadpoolCallback(void(__stdcall *callback)(void*, void* context), void* context, void*) noexcept;
winrt::impl::ptp_timer __stdcall WINRT_IMPL_CreateThreadpoolTimer(void(__stdcall *callback)(void*, void* context, void*), void* context, void*) noexcept;
int32_t __stdcall WINRT_IMPL_SetThreadpoolTimerEx(winrt::impl::ptp_timer timer, void* time, uint32_t period, uint32_t window) noexcept;
int32_t __stdcall WINRT_IMPL_SetThreadpoolTimer(winrt::impl::ptp_timer timer, void* time, uint32_t period, uint32_t window) noexcept;
void __stdcall WINRT_IMPL_CloseThreadpoolTimer(winrt::impl::ptp_timer timer) noexcept;
winrt::impl::ptp_wait __stdcall WINRT_IMPL_CreateThreadpoolWait(void(__stdcall *callback)(void*, void* context, void*, uint32_t result), void* context, void*) noexcept;
int32_t __stdcall WINRT_IMPL_SetThreadpoolWaitEx(winrt::impl::ptp_wait wait, void* handle, void* timeout, void* reserved) noexcept;
int32_t __stdcall WINRT_IMPL_SetThreadpoolWait(winrt::impl::ptp_wait wait, void* handle, void* timeout) noexcept;
void __stdcall WINRT_IMPL_CloseThreadpoolWait(winrt::impl::ptp_wait wait) noexcept;
winrt::impl::ptp_io __stdcall WINRT_IMPL_CreateThreadpoolIo(void* object, void(__stdcall *callback)(void*, void* context, void* overlapped, uint32_t result, std::size_t bytes, void*) noexcept, void* context, void*) noexcept;
void __stdcall WINRT_IMPL_StartThreadpoolIo(winrt::impl::ptp_io io) noexcept;
Expand Down Expand Up @@ -149,10 +149,10 @@ WINRT_IMPL_LINK(WaitForSingleObject, 8)

WINRT_IMPL_LINK(TrySubmitThreadpoolCallback, 12)
WINRT_IMPL_LINK(CreateThreadpoolTimer, 12)
WINRT_IMPL_LINK(SetThreadpoolTimerEx, 16)
WINRT_IMPL_LINK(SetThreadpoolTimer, 16)
WINRT_IMPL_LINK(CloseThreadpoolTimer, 4)
WINRT_IMPL_LINK(CreateThreadpoolWait, 12)
WINRT_IMPL_LINK(SetThreadpoolWaitEx, 16)
WINRT_IMPL_LINK(SetThreadpoolWait, 12)
WINRT_IMPL_LINK(CloseThreadpoolWait, 4)
WINRT_IMPL_LINK(CreateThreadpoolIo, 16)
WINRT_IMPL_LINK(StartThreadpoolIo, 4)
Expand Down