Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-111569: Fix critical sections test on WebAssembly#111897

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
brettcannon merged 3 commits intopython:mainfromcolesbury:critical_sections-wasm
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from1 commit
Commits
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
7 changes: 7 additions & 0 deletionsInclude/pyport.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -470,6 +470,13 @@ extern "C" {
# define WITH_THREAD
#endif

/* Some WebAssembly platforms do not provide a working pthread implementation.
* Thread support is stubbed and any attempt to create a new thread fails.
*/
#if !defined(__wasi__) && (!defined(__EMSCRIPTEN__) || defined(__EMSCRIPTEN_PTHREADS__))
# define Py_CAN_START_THREADS 1
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately it's trickier than that because youcan do a WASI build with experimental threading support (the "experimental" bit is on the WASI side, not our side); see the--with-wasm-pthreads for turning the support on.

cpython/configure.ac

Lines 2123 to 2127 in6f09f69

AS_VAR_IF([enable_wasm_pthreads],[yes],[
AS_VAR_APPEND([CFLAGS_NODIST],[" -pthread"])
AS_VAR_APPEND([LDFLAGS_NODIST],[" -sUSE_PTHREADS"])
AS_VAR_APPEND([LINKFORSHARED],[" -sPROXY_TO_PTHREAD"])
])

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@brettcannon, do you think the general approach here is a reasonable? Previously, I had been just checking for thread support on the Python side, but that's a bit awkward for these C API tests.

I'm a bit confused by your comment that you can do aWASI build with threading support. I see that there is Emscripten support for pthreads. That's already handled by the__EMSCRIPTEN_PTHREADS__ check on line 476.

The Python equivalent assumes thatWASI never supports threads:

elifsys.platform=="wasi":
returnFalse

I see that there is a WASI proposal for threads (https://github.com/WebAssembly/wasi-threads), but that looks like it's still marked "phase 1".

Copy link
Member

Choose a reason for hiding this comment

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

The Python equivalent assumes thatWASI never supports threads

The threading support isn't well-tested because I haven't set up a buildbot yet and made it priority to more thoroughly test it. 😅

I see that there is a WASI proposal for threads (https://github.com/WebAssembly/wasi-threads), but that looks like it's still marked "phase 1".

Yep, and it's staying at that phase because threading is being taken up by the WebAssembly CG at the W3C which acts as an upstream to WASI. That proposal hit I think phase 4 last month (it's definitely moving forward regardless of whether I got the number right), hence why this whole "threads in WASI" thing continues to be experimental both in WASI and for us.

I think we need to make a decision about free-threading and WASI. Do we want to try and make it work with the experimental threading support that's there, or do we say it's not worth it and wait on the more official support to land?

colesbury reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I think we need to make a decision about free-threading and WASI.

My 2¢ is "no, not for now", but I also think that's only tangentially related to this issue. The test breakage is not happening in a free-threaded (--disable-gil) build. Even if we don't support the combination of--disable-gil and wasm, I'd still like a way to determine at compile time (in C code) ifPyThread_start_new_thread will actually launch a thread (or is just a no-op stub).

I think we might be able to just useHAVE_PTHREAD_STUBS here instead of defining a newPy_CAN_START_THREADS.

I'm hoping to run this on the wasm buildbots, but I think the runner is backed up... probably because thetest_critical_sections_threads test hangs until the test suite times out.

Copy link
Member

Choose a reason for hiding this comment

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

👍 if theHAVE_PTHREADS_STUBS solution works.

Copy link
Member

Choose a reason for hiding this comment

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

And I think the buildbots have calmed down:https://buildbot.python.org/all/#/builders?tags=wasm

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Hmmm...HAVE_PTHREAD_STUBS did not seem to work.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It appears that when built without thread support WASI usesHAVE_PTHREAD_STUBS, but Emscripten stubs pthreads internally.


#ifdef WITH_THREAD
# ifdef Py_BUILD_CORE
# ifdef HAVE_THREAD_LOCAL
Expand Down
4 changes: 4 additions & 0 deletionsModules/_testinternalcapi/test_critical_sections.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -170,6 +170,7 @@ thread_critical_sections(void *arg)
}
}

#ifdef Py_CAN_START_THREADS
static PyObject *
test_critical_sections_threads(PyObject *self, PyObject *Py_UNUSED(args))
{
Expand All@@ -194,12 +195,15 @@ test_critical_sections_threads(PyObject *self, PyObject *Py_UNUSED(args))
Py_DECREF(test_data.obj1);
Py_RETURN_NONE;
}
#endif

static PyMethodDef test_methods[] = {
{"test_critical_sections", test_critical_sections, METH_NOARGS},
{"test_critical_sections_nest", test_critical_sections_nest, METH_NOARGS},
{"test_critical_sections_suspend", test_critical_sections_suspend, METH_NOARGS},
#ifdef Py_CAN_START_THREADS
{"test_critical_sections_threads", test_critical_sections_threads, METH_NOARGS},
#endif
{NULL, NULL} /* sentinel */
};

Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp