Propagate the ValueError when a value is not shareable.

python · ericsnowcurrently · Nov 1, 2023 · Oct 24, 2023 · Oct 25, 2023 · Oct 25, 2023
commit 45488f20dcc699099fb83db32c294d4ed1654433
@@ -172,6 +172,7 @@ typedef enum error_code {
     _PyXI_ERR_OTHER = -2,
     _PyXI_ERR_NO_MEMORY = -3,
     _PyXI_ERR_ALREADY_RUNNING = -4,
+    _PyXI_ERR_NOT_SHAREABLE = -5,
 } _PyXI_errcode;
 
 PyAPI_FUNC(int) _PyXI_ApplyErrorCode(
@@ -198,14 +199,18 @@ PyAPI_FUNC(void) _PyXI_ApplyExceptionInfo(
     PyObject *exctype);
 
 
+typedef struct xi_session _PyXI_session;
 typedef struct _sharedns _PyXI_namespace;
 
 PyAPI_FUNC(void) _PyXI_FreeNamespace(_PyXI_namespace *ns);
 PyAPI_FUNC(_PyXI_namespace *) _PyXI_NamespaceFromNames(PyObject *names);
-PyAPI_FUNC(_PyXI_namespace *) _PyXI_NamespaceFromDict(PyObject *nsobj);
+PyAPI_FUNC(_PyXI_namespace *) _PyXI_NamespaceFromDict(
+    PyObject *nsobj,
+    _PyXI_session *session);
 PyAPI_FUNC(int) _PyXI_FillNamespaceFromDict(
     _PyXI_namespace *ns,
-    PyObject *nsobj);
+    PyObject *nsobj,
+    _PyXI_session *session);
 PyAPI_FUNC(int) _PyXI_ApplyNamespace(
     _PyXI_namespace *ns,
     PyObject *nsobj,
@@ -222,7 +227,7 @@ PyAPI_FUNC(int) _PyXI_ApplyNamespace(
 // isolation between interpreters.  This includes setting objects
 // in the target's __main__ module on the way in, and capturing
 // uncaught exceptions on the way out.
-typedef struct xi_session {
+struct xi_session {
     // Once a session has been entered, this is the tstate that was
     // current before the session.  If it is different from cur_tstate
     // then we must have switched interpreters.  Either way, this will
@@ -253,12 +258,13 @@ typedef struct xi_session {
 
     // -- pre-allocated memory --
     _PyXI_exception_info _exc;
-} _PyXI_session;
+    _PyXI_errcode _exc_override;
+};
 
 PyAPI_FUNC(int) _PyXI_Enter(
+    _PyXI_session *session,
     PyInterpreterState *interp,
-    PyObject *nsupdates,
-    _PyXI_session *session);
+    PyObject *nsupdates);
 PyAPI_FUNC(void) _PyXI_Exit(_PyXI_session *session);
 
 PyAPI_FUNC(void) _PyXI_ApplyCapturedException(

diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c
@@ -235,7 +235,7 @@ _run_in_interpreter(PyInterpreterState *interp,
     _PyXI_session session = {0};
 
     // Prep and switch interpreters.
-    if (_PyXI_Enter(interp, shareables, &session) < 0) {
+    if (_PyXI_Enter(&session, interp, shareables) < 0) {
         assert(!PyErr_Occurred());
         _PyXI_ApplyExceptionInfo(session.exc, excwrapper);
         assert(PyErr_Occurred());

@@ -192,11 +192,16 @@ _PyCrossInterpreterData_Lookup(PyObject *obj)
 }
 
 static inline void
-_set_xid_lookup_failure(PyInterpreterState *interp, PyObject *obj)
+_set_xid_lookup_failure(PyInterpreterState *interp,
+                        PyObject *obj, const char *msg)
 {
     PyObject *exctype = interp->xi.PyExc_NotShareableError;
     assert(exctype != NULL);
-    if (obj == NULL) {
+    if (msg != NULL) {
+        assert(obj == NULL);
+        PyErr_SetString(exctype, msg);
+    }
+    else if (obj == NULL) {
         PyErr_SetString(exctype,
                         "object does not support cross-interpreter data");
     }
@@ -213,7 +218,7 @@ _PyObject_CheckCrossInterpreterData(PyObject *obj)
     crossinterpdatafunc getdata = _lookup_getdata(interp, obj);
     if (getdata == NULL) {
         if (!PyErr_Occurred()) {
-            _set_xid_lookup_failure(interp, obj);
+            _set_xid_lookup_failure(interp, obj, NULL);
         }
         return -1;
     }
@@ -240,7 +245,7 @@ _PyObject_GetCrossInterpreterData(PyObject *obj, _PyCrossInterpreterData *data)
     if (getdata == NULL) {
         Py_DECREF(obj);
         if (!PyErr_Occurred()) {
-            _set_xid_lookup_failure(interp, obj);
+            _set_xid_lookup_failure(interp, obj, NULL);
         }
         return -1;
     }
@@ -719,6 +724,9 @@ _PyXI_ApplyErrorCode(_PyXI_errcode code, PyInterpreterState *interp)
         assert(_PyInterpreterState_IsRunningMain(interp));
         _PyInterpreterState_FailIfRunningMain(interp);
         break;
+    case _PyXI_ERR_NOT_SHAREABLE:
+        _set_xid_lookup_failure(interp, NULL, NULL);
+        break;
     default:
 #ifdef Py_DEBUG
         Py_UNREACHABLE();
@@ -778,6 +786,10 @@ _PyXI_ApplyExceptionInfo(_PyXI_exception_info *info, PyObject *exctype)
         // Raise an exception that proxies the propagated exception.
         _Py_excinfo_Apply(&info->uncaught, exctype);
     }
+    else if (info->code == _PyXI_ERR_NOT_SHAREABLE) {
+        // Propagate the exception directly.
+        _set_xid_lookup_failure(info->interp, NULL, info->uncaught.msg);
+    }
     else {
         // Raise an exception corresponding to the code.
         assert(info->code != _PyXI_ERR_NO_ERROR);
@@ -831,6 +843,8 @@ _sharednsitem_set_value(_PyXI_namespace_item *item, PyObject *value)
             PyMem_RawFree(item->data);
         }
         item->data = NULL;
+        // The caller may want to propagate PyExc_NotShareableError
+        // if currently switched between interpreters.
         return -1;
     }
     return 0;
@@ -1007,9 +1021,14 @@ _PyXI_NamespaceFromNames(PyObject *names)
     return ns;
 }
 
+static void _propagate_not_shareable_error(_PyXI_session *);
+
+// All items are expected to be shareable.
 _PyXI_namespace *
-_PyXI_NamespaceFromDict(PyObject *nsobj)
+_PyXI_NamespaceFromDict(PyObject *nsobj, _PyXI_session *session)
 {
+    // session must be entered already, if provided.
+    assert(session == NULL || session->init_tstate != NULL);
     if (nsobj == NULL || nsobj == Py_None) {
         return NULL;
     }
@@ -1034,30 +1053,37 @@ _PyXI_NamespaceFromDict(PyObject *nsobj)
     for (Py_ssize_t i=0; i < len; i++) {
         PyObject *key, *value;
         if (!PyDict_Next(nsobj, &pos, &key, &value)) {
-            break;
+            goto error;
         }
         _PyXI_namespace_item *item = &ns->items[i];
         if (_sharednsitem_init(item, interpid, key) != 0) {
-            break;
+            goto error;
         }
         if (_sharednsitem_set_value(item, value) < 0) {
             _sharednsitem_clear(item);
-            break;
+            _propagate_not_shareable_error(session);
+            goto error;
         }
     }
-    if (PyErr_Occurred()) {
-        _PyXI_FreeNamespace(ns);
-        return NULL;
-    }
     return ns;
+
+error:
+    assert(PyErr_Occurred()
+           || (session != NULL && session->exc_override != NULL));
+    _PyXI_FreeNamespace(ns);
+    return NULL;
 }
 
 int
-_PyXI_FillNamespaceFromDict(_PyXI_namespace *ns, PyObject *nsobj)
+_PyXI_FillNamespaceFromDict(_PyXI_namespace *ns, PyObject *nsobj,
+                            _PyXI_session *session)
 {
+    // session must be entered already, if provided.
+    assert(session == NULL || session->init_tstate != NULL);
     for (Py_ssize_t i=0; i < ns->len; i++) {
         _PyXI_namespace_item *item = &ns->items[i];
         if (_sharednsitem_copy_from_ns(item, nsobj) < 0) {
+            _propagate_not_shareable_error(session);
             // Clear out the ones we set so far.
             for (Py_ssize_t j=0; j < i; j++) {
                 _sharednsitem_clear_data(&ns->items[j]);
@@ -1151,6 +1177,20 @@ _exit_session(_PyXI_session *session)
     session->init_tstate = NULL;
 }
 
+static void
+_propagate_not_shareable_error(_PyXI_session *session)
+{
+    if (session == NULL) {
+        return;
+    }
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (PyErr_ExceptionMatches(interp->xi.PyExc_NotShareableError)) {
+        // We want to propagate the exception directly.
+        session->_exc_override = _PyXI_ERR_NOT_SHAREABLE;
+        session->exc_override = &session->_exc_override;
+    }
+}
+
 static void
 _capture_current_exception(_PyXI_session *session)
 {
@@ -1172,6 +1212,9 @@ _capture_current_exception(_PyXI_session *session)
         // We want to actually capture the current exception.
         excval = PyErr_GetRaisedException();
     }
+    else if (errcode == _PyXI_ERR_NOT_SHAREABLE) {
+        // We will set the errcode, in addition to capturing the exception.
+    }
     else {
         // We could do a variety of things here, depending on errcode.
         // However, for now we simply ignore the exception and rely
@@ -1229,13 +1272,13 @@ _PyXI_HasCapturedException(_PyXI_session *session)
 }
 
 int
-_PyXI_Enter(PyInterpreterState *interp, PyObject *nsupdates,
-            _PyXI_session *session)
+_PyXI_Enter(_PyXI_session *session,
+            PyInterpreterState *interp, PyObject *nsupdates)
 {
     // Convert the attrs for cross-interpreter use.
     _PyXI_namespace *sharedns = NULL;
     if (nsupdates != NULL) {
-        sharedns = _PyXI_NamespaceFromDict(nsupdates);
+        sharedns = _PyXI_NamespaceFromDict(nsupdates, NULL);
         if (sharedns == NULL && PyErr_Occurred()) {
             assert(session->exc == NULL);
             return -1;