homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author aeros
Recipients aeros, eric.snow, vstinner
Date 2019年09月16日.06:48:15
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1568616495.46.0.492892358093.issue37224@roundup.psfhosted.org>
In-reply-to
Content
Upon further reading of the documentation and some experimentation, it would definitely not make sense to call `PyInterpreterState_Delete` here (since we're only closing the sub-interpreter in the current thread), so that's not the issue. I still have no idea as to why it's commented out there, but that's besides the point.
I think I may have found the potential cause of the test failure though. Looking into `test_still_running()`, it's clear that the intention is for a RuntimeError to be raised if `interpreters.destroy()` is called on a currently running interpreter:
```
 def test_still_running(self):
 main, = interpreters.list_all()
 interp = interpreters.create()
 with _running(interp):
 with self.assertRaises(RuntimeError):
 interpreters.destroy(interp)
 self.assertTrue(interpreters.is_running(interp))
```
However, within interp_destroy (https://github.com/python/cpython/blob/56a45142e70a1ccf3233d43cb60c47255252e89a/Modules/_xxsubinterpretersmodule.c#L2024), it is apparent that the RuntimeError is ONLY raised when attempting to destroy the current interpreter:
```
 if (interp == current) {
 PyErr_SetString(PyExc_RuntimeError,
 "cannot destroy the current interpreter");
 return NULL;
 }
```
When attempting to destroy a running interpreter, NULL is returned without raising the RuntimeError: 
```
 if (_ensure_not_running(interp) < 0) {
 return NULL;
 }
```
This was my first guess at a solution:
```
 if (_ensure_not_running(interp) < 0) {
 PyErr_SetString(PyExc_RuntimeError,
 "cannot destroy a running interpreter")
 return NULL;
 }
```
But, within `_ensure_not_running()` (https://github.com/python/cpython/blob/56a45142e70a1ccf3233d43cb60c47255252e89a/Modules/_xxsubinterpretersmodule.c#L1842), a RuntimeError is supposed to be raised from:
```
 if (is_running) {
 PyErr_Format(PyExc_RuntimeError, "interpreter already running");
 return -1;
 }
```
Initially, I was unsure of the difference between `PyErr_SetString()` and `PyErr_Format()`, so I referred to the documentation. `PyErr_Format()` is similar, but it also returns NULL. If I'm not mistaken doesn't mean that the `return -1` within the `if (is_running)` bock is effectively ignored? If so, this would potentially explain the problem... 
I think the appropriate solution would be to replace `PyErr_Format()` with `PyErr_SetString()` within `_ensure_not_running()`. Also, I think it would be more useful to additionally raise the RuntimeError within `interp_destroy()` if the interpreter is running, to provide a more useful and specific error message. "cannot destroy a running interpreter" is far more useful for debugging purposes than a more generic "interpreter already running".
I plan on opening a PR to make these changes in the next few days. Let me know what you think Victor.
History
Date User Action Args
2019年09月16日 06:48:15aerossetrecipients: + aeros, vstinner, eric.snow
2019年09月16日 06:48:15aerossetmessageid: <1568616495.46.0.492892358093.issue37224@roundup.psfhosted.org>
2019年09月16日 06:48:15aeroslinkissue37224 messages
2019年09月16日 06:48:15aeroscreate

AltStyle によって変換されたページ (->オリジナル) /