Skip to content

Commit

Permalink
Remove hack to propagate watchers to sub-interpreters
Browse files Browse the repository at this point in the history
Summary:
We really don't support sub-interpreters properly with Cinder, and any usage with threads will likely break. I also can't see how to make this work with CinderX as a module without new functionality in core Python. I'm not sure how much we'll support sub-interpreters in future anyway so let's not pretend they work today.

I suspect carljm added this originally to make tests pass. So, instead I've setup the broken tests to skip instead.

Reviewed By: oclbdk

Differential Revision: D50663830

fbshipit-source-id: 7bd114b75aedcd1107815f23084cc655f8c5fca2
  • Loading branch information
jbower-fb authored and facebook-github-bot committed Oct 30, 2023
1 parent cf9ee67 commit 1d34ac9
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 64 deletions.
7 changes: 0 additions & 7 deletions Cinder/Include/cinder/cinder.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ PyAPI_FUNC(int) Cinder_Init(void);
*/
PyAPI_FUNC(int) Cinder_Fini(void);

/*
* Initialize per-subinterpreter Cinder state.
*
* Returns 0 on success or -1 on error.
*/
PyAPI_FUNC(int) Cinder_InitSubInterp(void);

/*
* Watch or unwatch a dictionary.
*/
Expand Down
50 changes: 0 additions & 50 deletions Cinder/cinder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,53 +209,3 @@ int Cinder_Fini() {
_PyClassLoader_ClearCache();
return _PyJIT_Finalize();
}

int Cinder_InitSubInterp() {
// HACK: for now we assume we are the only watcher out there, so that we can
// just keep track of a single watcher ID rather than one per interpreter.
int prev_dict_watcher_id = cinder_dict_watcher_id;
JIT_CHECK(
prev_dict_watcher_id >= 0,
"Initializing sub-interpreter without main interpreter?");
if (cinder_install_dict_watcher() < 0) {
return -1;
}
JIT_CHECK(
cinder_dict_watcher_id == prev_dict_watcher_id,
"Somebody else watching dicts?");

int prev_type_watcher_id = cinder_type_watcher_id;
JIT_CHECK(
prev_type_watcher_id >= 0,
"Initializing sub-interpreter without main interpreter?");
if (cinder_install_type_watcher() < 0) {
return -1;
}
JIT_CHECK(
cinder_type_watcher_id == prev_type_watcher_id,
"Somebody else watching types?");

int prev_func_watcher_id = cinder_func_watcher_id;
JIT_CHECK(
prev_func_watcher_id >= 0,
"Initializing sub-interpreter without main interpreter?");
if (cinder_install_func_watcher() < 0) {
return -1;
}
JIT_CHECK(
cinder_func_watcher_id == prev_func_watcher_id,
"Somebody else watching functions?");

int prev_code_watcher_id = cinder_code_watcher_id;
JIT_CHECK(
prev_code_watcher_id >= 0,
"Initializing sub-interpreter without main interpreter?");
if (cinder_install_code_watcher() < 0) {
return -1;
}
JIT_CHECK(
cinder_code_watcher_id == prev_code_watcher_id,
"Somebody else watching code objects?");

return 0;
}
13 changes: 13 additions & 0 deletions CinderX/TestScripts/cinder_skip_test.txt
Original file line number Diff line number Diff line change
@@ -1 +1,14 @@
test.test_rlcompleter.TestRlcompleter.test_global_matches


test.test__xxsubinterpreters.*
test.test_ast.ModuleStateTests.test_subinterpreter
test.test_atexit.SubinterpreterTest.*
test.test_capi.SubinterpreterTest.*
test.test_embed.AuditingTests.test_audit_subinterpreter
test.test_embed.EmbeddingTests.test_subinterps_different_ids
test.test_embed.EmbeddingTests.test_subinterps_distinct_state
test.test_embed.EmbeddingTests.test_subinterps_main
test.test_multibytecodec.Test_IncrementalEncoder.test_subinterp
test.test_threading.SubinterpThreadingTests.*
test.test_interpreters.*
7 changes: 0 additions & 7 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -2082,13 +2082,6 @@ new_interpreter(PyThreadState **tstate_p, int isolated_subinterpreter)
goto error;
}

#ifdef ENABLE_CINDERX
int cinder_status = Cinder_InitSubInterp();
if (cinder_status < 0) {
goto error;
}
#endif

status = pycore_interp_init(tstate);
if (_PyStatus_EXCEPTION(status)) {
goto error;
Expand Down

0 comments on commit 1d34ac9

Please sign in to comment.