[winrt support] Enabling Boost.Thread to be used in the Windows Runtime. by stgates · Pull Request #18 · boostorg/thread (original) (raw)

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.

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

Already on GitHub?Sign in to your account

Conversation42 Commits8 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This involves basically 3 changes:

1. Using __declspec(thread) instead of the Tls APIs.
2. Using Windows::System::Threading since Win32 Threading APIs aren't allowed.
3. Updating or replacing some banned APIs like WaitForSingleObject with WaitForSingleObjectEx.

This involves basically 3 changes:

1. Using __declspec(thread) instead of the Tls APIs.
2. Using Windows::System::Threading since Win32 Threading APIs aren't allowed.
3. Updating or replacing some banned APIs like WaitForSingleObject with WaitForSingleObjectEx.

I'll try to review this this weekend.

BTW do you have a method for building on WinRT? i.e. have you patched Boost.Build for it?

Yes I have a pending pull request (boostorg/build#14) on Boost.Build for targeting Windows Store and Windows Phone.

Thanks for taking a look,
Steve

Choose a reason for hiding this comment

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

The (C) Microsoft may cause problems for the steering committee. As you will note throughout all Boost code, we don't copyright non-individuals, what happens is that the individual is working for whatever corporate employer but gets approval to personally take the copyright. Would Microsoft be able to give you personally the copyright? I know Eric Niebler used to work for Microsoft, he may be able to advise on what Microsoft channels to use.

Choose a reason for hiding this comment

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

I talked again with my legal group and they've agreed to allow the contributions without the Microsoft copyright :). So I removed them from all the source files.

If you can clean up the ifdef splosh significantly, this patch looks to be of good enough quality it can be merged. Thanks for taking the effort to submit it.

Reverting disabling thread attributes for WinRT. Created common GetSystemInfo/GetNativeSystemInfo function. Fix this_thread get_id() bug on WinRT. Enabled initializing the Windows Runtime in each test for execution. This is not when using in Windows store/phone applications, just if a desktop app.

Hi Niall,

I think I've addressed all of our comments now. Let me know if you have any additional feedback.

Thanks,
Steve

Choose a reason for hiding this comment

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

I assume this reorder is due to a bad find in files and replace?

Choose a reason for hiding this comment

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

I'm not exactly sure where from but I can fix it back up.

Choose a reason for hiding this comment

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

A number of copyright headers seem to have become reordered. Weird. Anyway, apart from that this patch looks good enough for me to try building. Unfortunately for you we have the first beta of Boost v1.56 in the works, and this Saturday will be lost on getting my CI soak testing code for 24 hours and other such fun. If no nasty surprises turn up, I'll get to this the following Saturday, but if I miss that date I'm on a business trip the following weekend so it could potentially be a month out before I get back to this. Is this okay?

Choose a reason for hiding this comment

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

I'll fix up the copyright header ordering.

Greedily from my side the sooner this is in the better :), but I understand. Let me know if I can do anything or you need more information.

Copyright reordering has been fixed. There is no action on my side now until the GetTickCount64 merges.

Conflicts: include/boost/thread/win32/thread_primitives.hpp src/win32/gettickcount64.cpp

Ok I merged with the GetTickCount changes, basically there is no choice but to use GetTickCount64 for the Windows Runtime.

Yup seems reasonable. Thanks.

Niall, could this PR be merged?

I have testing it scheduled for the coming Saturday, so one week from today. Today was spent upgrading website software for security bugs, and probably so will tomorrow.

I've got this patch compiling, but the tests won't run on my Win8.1 machine (x86). Am I missing something?

b2 libs/thread/test windows-api=store

Performing configuration checks

    - symlinks supported       : no  (cached)
    - junctions supported      : yes (cached)
    - hardlinks supported      : yes (cached)
...patience...
...patience...
...patience...
...found 12368 targets...
...updating 1409 targets...
testing.capture-output bin.v2\libs\thread\test\packaged_task__move_asign_p.test\msvc-12.0\debug\threading-multi\windows-api-store\packaged_task__move_asign_p.run
====== BEGIN OUTPUT ======
The system cannot execute the specified program.

EXIT STATUS: 9020 
====== END OUTPUT ======

Thanks for merging.

Yes the tests for WinRT can't run yet because they require the pending changes I have on Boost.Build and Boost.Test. With both of those pull requests the tests can be run.

boostorg/build#26
boostorg/test#9

@stgates I merged the Build changes locally, so the above reflects that. Also, I believe Thread mostly uses the lightweight Boost.Test alternative, and therefore doesn't need Boost.Test.

If you look closely at my build log above, it looks to me that the build outputs binaries which cannot run on a 32-bit Windows 8 machine. Perhaps I need x64?

Would these versions be compatible with or useable for developing an Xbox app, using the DurangoADK, that has legacy C++ code that needs Boost for threads, signals, ...?

If not, are there any alternatives?

@jgleitsmann I'm not very familiar for developing for the Xbox, but potentially it could work.