Add python excepiton handling catch block to resolve deadlock #35283
Conversation
Differential Revision: D7753253 Differential Version: 100740035
xush6528
requested review from
mrshenli,
pritamdamania87 and
zhaojuanmao
as code owners
💊 CircleCI build failures summary and remediationsAs of commit f3f23bf (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no CircleCI failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker. This comment has been revised 1 time. |
| @@ -477,6 +477,16 @@ std::shared_ptr<FutureMessage> RequestCallbackImpl::processMessage( | |||
| // processMessage(). | |||
| ClearAutogradContextGuard guard; | |||
| processRpc(*rpc, messageType, id, retFuture); | |||
| } catch (py::error_already_set& e) { | |||
There was a problem hiding this comment.
Is this the only error type we gonna get from Python?
There was a problem hiding this comment.
Yes, this is pybind implementation. If C++ calls pybind, and a Python exception is thrown and not handled in Python, this is the only exception that will be thrown to C++ land.
There was a problem hiding this comment.
what is the scope of things that can be called from procesRpc? We don't always call C++ through pybind in PyTorch.
|
while this time the Python exception is thrown from So this PR is a catch-call fix for all possible pybind python functions called in RPC server-side request handlers. |
xush6528
added
the
module: rpc
|
This pull request has been merged in 36e3c00. |
…h#35283) Summary: Note: This PR has been merged into master after the 1.5.0 branch cut at 36e3c00 (see original PR: pytorch#35283). This PR is to cherry pick it into 1.5. ---- Original Commit Description Follows --- Pull Request resolved: pytorch#35283 pytorch#34260 Deadlock on destructing py::error_already_set. There are request callback impls in Python, where Python exceptions could be thrown. For releasing Python exception py::objects, GIL must be held. Differential Revision: D7753253 fbshipit-source-id: 4bfaaaf027e4254f5e3fedaca80228c8b4282e39
#35402) Summary: Note: This PR has been merged into master after the 1.5.0 branch cut at 36e3c00 (see original PR: #35283). This PR is to cherry pick it into 1.5. ---- Original Commit Description Follows --- Pull Request resolved: #35283 #34260 Deadlock on destructing py::error_already_set. There are request callback impls in Python, where Python exceptions could be thrown. For releasing Python exception py::objects, GIL must be held. Differential Revision: D7753253 fbshipit-source-id: 4bfaaaf027e4254f5e3fedaca80228c8b4282e39 Co-authored-by: Shihao Xu <shihaoxu@fb.com>
Stack:
:black_circle: #35283 Add python excepiton handling catch block to resolve deadlock 💛
#34260
Deadlock on destructing py::error_already_set.
There are request callback impls in Python, where Python exceptions could be thrown. For releasing Python exception py::objects, GIL must be held.
Differential Revision: D7753253