windows/thread: Add support for win32 micropython _thread.

[andrewleech]

This PR adds support for thread to the windows port with both mingw and msvc compilers.

Passes the test suite:

C:\Users\andrewleech\src\micropython\tests>py run-tests.py -d thread
pass  thread/mutate_bytearray.py
pass  thread/mutate_instance.py
pass  thread/stress_recurse.py
skip  thread/stress_schedule.py
pass  thread/thread_exc1.py
pass  thread/mutate_list.py
pass  thread/mutate_dict.py
pass  thread/thread_gc1.py
pass  thread/thread_heap_lock.py
pass  thread/thread_ident1.py
pass  thread/thread_lock1.py
pass  thread/mutate_set.py
pass  thread/stress_create.py
pass  thread/thread_exc2.py
pass  thread/thread_lock5.py
pass  thread/thread_lock3.py
pass  thread/thread_shared1.py
pass  thread/thread_shared2.py
pass  thread/thread_exit1.py
pass  thread/thread_stacksize1.py
pass  thread/thread_exit2.py
pass  thread/thread_lock2.py
pass  thread/thread_lock4.py
pass  thread/thread_qstr1.py
pass  thread/thread_start1.py
pass  thread/thread_start2.py
pass  thread/stress_heap.py
pass  thread/thread_sleep1.py
pass  thread/stress_aes.py
28 tests performed (141 individual testcases)
28 tests passed
1 tests skipped: stress_schedule

[stinos]

Looks good. The thread tests don't run in CI though, would be best to add that.

thread_exc2 fails here for all msvc builds:

@@ -1,5 +1,5 @@
-Unhandled exception in thread started by <function thread_entry at 0x\[0-9a-f\]\+>
+Unhandled exception in thread started by <function thread_entry at 0x3061a0>
 Traceback (most recent call last):
-  File \.\+, line 7, in thread_entry
+  File "./thread/thread_exc2.py", line 7, in thread_entry
 ValueError:
 done

and thread_thread_stacksize1.py hangs forever for Debug/x64 but does print

0
True
0

For ease of maintainance it's tempting to incorporate this into unix/mpthreadport though since the main difference is pthread vs win32 and the rest of the code is non-trivial, have you considered that?

[andrewleech]

That's curious, the unit test results above were from a local msvc build on my laptop, I wonder why there's a difference.

I guess I could possibly turn what I've done into a custom wrapper/shim to work with the unix driver, not sure how much value is in that now though.

I didn't previously think it was worth it, there really are a fair few differences - the signalling method for thread gc needed a new architecture, the entry needed a wrapper and the waiting for events is a common function vs different functions for each type of object.
I also needed to extend the _thread struct used for the linked list.

I had previously tried using a win32-pthreads
(wrapper style) library to provide compatibility with the existing unix driver but that wasn't trivial either, didn't resolve the signalling issue and brought in more dependencies.

[andrewleech]

Ah, thread_exc2 is in a special_tests category that is skipped by default - I'm not entirely sure why.

In terms of ci - yeah it turns out the thread tests aren't run by default on any platform I think - it's not run on unix port ci tests at least! Again, it seems like something that should be run on any platform that supports it...

micropython/tests/run-tests.py

Line 806 in 2a7b6d6

if len(args.files) == 0:

[stinos]

Yeah i wouldn't go for win32-pthreads especially if you have something which works already.

[stinos]

I'd think MicroPython's policy is to not check in dead code?

[andrewleech]

Whoops yeah, thanks for that. I initially expected to need MICROPY_PY_THREAD to be defined in makefile snippet like other ports, ended up not turning out that way as it doesn't need extra linker argurments.

I did push this branch along with the other windows ones I'm working on in a rush before the meetup so didn't do a personal git review properly... sorry for the extra noise :-/

[stinos]

Minor nitpick; perhaps this should be enforced by the formatting as well, but it looks a bit off to have half of the comments like // This is a comment. and the others // this is a comment

[andrewleech]

That's a good point really, I guess it's possibly asking too much to parse for grammer in the auto format checker, but things like this all contribute to the consistency of quality in a project.

[andrewleech]

FWIW my main use case for threads on unix/windows is to enable easy mocking of Timers and similar for running unit tests, a loop in a Thread behaves quite similar to an interrupt in embedded code,m especially if you enforce a gc lock in the mocked function.

[stinos]

easy mocking of Timers

I assume you're aware that most likely all sleep/wait implementations on windows by default use a 16mSec (or close to that) resolution? Just mentioning because it's a common source of problems in tests which deal with timing.

[andrewleech]

I didn't realise that, do you know if that applies to the SleepEx command I've used here?
I assumed it'd be more efficient than the custom usleep function, though if SleepEx can't do better than 16ms resolution perhaps I should go back to using usleep (assuming it does work as described).

My mock timers are generally less about testing specific exact timing (it's always hard to compare embedded to unix for timing) but more about being able to run the same code on embedded as on desktop and have background tasks run regularly that can interrupt the main application at any point (unlike background async tasks).

[stinos]

do you know if that applies to the SleepEx command I've used here?

Yes, see https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sleepex#remarks

My mock timers are generally less about testing specific exact timing

Yeah as long as there's nothing which explicitly relies on wait/sleep being accurate (there shouldn't be ideally) it is usually ok, but still, especially with smaller timeouts, things sometimes don't go well. But in any cas: if tests pass there's not much to worry about.

[dpgeorge]

@andrewleech can you please rebase this on latest master? And also enable thread tests in ports/windows/.appveyor.yml by adding thread to the end of line 56.

[dpgeorge]

spelling: "semaphore"

[dpgeorge]

Is this the right way to do things? If usleep is #define'd to something then that define will be active in all other compilation units. So only the code in this file will pick up this custom usleep implementation.

Why not just keep it as usleep_impl()?

[andrewleech]

Yeah that was a bit strange / wrong. usleep_impl() was only being used in the single usleep() wrapper function below on msvc, it was ignored elsewhere. I thought I needed it to be the usleep used everywhere - but this wouldn't have actually been working as you suggest.

Turns out we don't need to provide usleep in any particular global sense, it was only being used in unix/mpconfigport.h to provide mp_hal_delay_us() so I think it's better to directly provide mp_hal_delay_us() here for win32 only.

[dpgeorge]

This change has broken some tests on AppVeyor:

1 tests failed: utime_time_ns
--- "C:/projects/micropython/tests/results\\extmod_utime_time_ns.py.exp"	2022-02-07 23:35:19.943524600 +0000
+++ "C:/projects/micropython/tests/results\\extmod_utime_time_ns.py.out"	2022-02-07 23:35:19.944525900 +0000
@@ -1,2 +1,2 @@
-True
-True
+False
+1644276919941525000 1644276919941525000 0
FAILURE C:/projects/micropython/tests/results\extmod_utime_time_ns.py

[projectgus]

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.