Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test3026: add support for Windows using native Win32 threads #9012

Closed
wants to merge 1 commit into from

Conversation

mback2k
Copy link
Member

@mback2k mback2k commented Jun 14, 2022

Also make sure the result is initialized to an invalid value (CURL_LAST) before the corresponding thread is launched.

Follow up to #8996

@mback2k mback2k added tests Windows Windows-specific labels Jun 14, 2022
@mback2k mback2k self-assigned this Jun 14, 2022
@mback2k mback2k force-pushed the test-3026-windows branch 2 times, most recently from c750e3f to d2510b4 Compare June 14, 2022 20:06
mback2k added a commit to mback2k/curl that referenced this pull request Jun 16, 2022
@mback2k mback2k changed the title tests/libtest/lib3026.c: try to implement test for Windows test3026: add support for Windows using native Win32 threads Jun 16, 2022
@mback2k mback2k requested review from jay, vszakats and bagder June 16, 2022 19:41
@mback2k mback2k marked this pull request as ready for review June 16, 2022 19:41
@mback2k
Copy link
Member Author

mback2k commented Jun 17, 2022

It seems like the threadsafe init is broken on 32-Bit Windows.

@jay
Copy link
Member

jay commented Jun 17, 2022

It seems like the threadsafe init is broken on 32-Bit Windows.

All the CI failures I see are errno 8 (ERROR_NOT_ENOUGH_MEMORY). Creating 1000 test threads is probably too many. Which CI are you referring to? You could try changing the stack size? Also I think on error it should say GetLastError instead of errno since it's actually an error code from the former.

@bagder
Copy link
Member

bagder commented Jun 17, 2022

Creating 1000 test threads is probably too many

If that is a problem, we can probably consider it tested with 100 too...

@mback2k
Copy link
Member Author

mback2k commented Jun 17, 2022

Yeah, I will try to reduce the number of threads in case a 32-bit build is done.

@mback2k mback2k marked this pull request as draft June 20, 2022 22:01
jay added a commit that referenced this pull request Jun 21, 2022
Follow-up to recent commits which added thread-safety support.

Bug: #9012 (comment)
Reported-by: Marc Hörsken

Closes #xxxx
jay added a commit that referenced this pull request Jun 21, 2022
Follow-up to recent commits which added thread-safety support.

Bug: #9012 (comment)
Reported-by: Marc Hörsken

Closes #9030
mback2k added a commit to mback2k/curl that referenced this pull request Jun 25, 2022
Reviewed-by: Viktor Szakats
Reviewed-by: Jay Satiro
Reviewed-by: Daniel Stenberg

Follow up to 7ade9c5
Closes curl#9012
Reviewed-by: Viktor Szakats
Reviewed-by: Jay Satiro
Reviewed-by: Daniel Stenberg

Follow up to 7ade9c5
Closes curl#9012
@mback2k
Copy link
Member Author

mback2k commented Jul 25, 2022

CI failures are unrelated. I think this is ready for merge. What do you think @bagder @jay @vszakats ?

@mback2k mback2k marked this pull request as ready for review July 25, 2022 09:55
@vszakats vszakats self-requested a review July 25, 2022 11:44
@mback2k mback2k closed this in 40b6206 Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Windows Windows-specific
Development

Successfully merging this pull request may close these issues.

None yet

4 participants