Check for null method in module execute #9713
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/9713
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit e133b68 with merge base 5098808 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D71968421 |
Summary: This is an attempt to partially mitigate use after free crashes when a module is unloaded during inference. This is not a supported use pattern, but it doesn't to sanity check here to avoid a crash in certain low memory shutdown scenarios. Differential Revision: D71968421
GregoryComer
force-pushed
the
export-D71968421
branch
from
8471ddd to
e133b68
Compare
|
This pull request was exported from Phabricator. Differential Revision: D71968421 |
| auto& method = methods_.at(method_name).method; | ||
| auto& inputs = methods_.at(method_name).inputs; | ||
|
|
||
| ET_CHECK_OR_RETURN_ERROR(method != nullptr, InvalidArgument, "Method is null"); |
There was a problem hiding this comment.
Trying to wrap my head how it could be possible at all that a unique_ptr held in an unordered_map would ever become nullptr after being once initialized in the past?
It's guaranteed the pointer isn't null by the time this code runs due to load_method() calls, then even if the Module used after deallocation, no code (like destructors) ever nullifies its members, so if the method is found in the dict, it wouldn't be nullptr.
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
|
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
3 participants
Summary: This is an attempt to partially mitigate use after free crashes when a module is unloaded during inference. This is not a supported use pattern, but it doesn't to sanity check here to avoid a crash.
Differential Revision: D71968421