[onert] Ensure proper pipeline cleanup on exceptions and shutdown#16379
[onert] Ensure proper pipeline cleanup on exceptions and shutdown#16379batcheu wants to merge 1 commit intoSamsung:masterfrom
Conversation
|
It is related below comments by @ragmani. |
|
@ragmani |
| } | ||
| catch (const std::exception &e) | ||
| { | ||
| _pipeline_manager->shutdown(); |
There was a problem hiding this comment.
From now on, any of exceptions including 'Asynchronous buffer filling' and 'Model execution' will call shutdown() explicitly to release resources properly.
There was a problem hiding this comment.
I'm not sure if BulkPipelineManager::shutdown() is always guaranteed not to throw exceptions. However, the current pipeline are already complex, and I'm don't know how to resolve this issue. So, I want to ignore this and move on to the next stage.
There was a problem hiding this comment.
There's no case of throwing additional exception in BulkPipelineManager::shutdown(). And even if an exception occurrs, it will abandon the previous one and proceed to the next one. 😊
| int nr_fclose_calls = 0; | ||
| EXPECT_TRUE(manager->initialize()); | ||
| // This hook will checking the number of fclose() calls | ||
| MockSyscallsManager::getInstance().setFcloseHook([&nr_fclose_calls](FILE *) -> int { | ||
| nr_fclose_calls++; | ||
| return 0; | ||
| }); | ||
| manager->shutdown(); | ||
| EXPECT_FALSE(manager->isInitialized()); | ||
| // fclose() should be called as the same number of models | ||
| EXPECT_EQ(nr_fclose_calls, nr_models); |
There was a problem hiding this comment.
It is difficult to test the manipulated failure of BulkPipelineBuffer:startAsyncBufferFill(). 😢
However, it is possible to indirectly verify whether resources are released when an issue arises by confirming whether the resources of BulkPipelineBuffer are released upon calling BufferPipelineManager::shutdown().
These changes ensure that pipeline resources are properly released even in error scenarios or when the pipeline was not fully initialized, preventing potential resource leaks. ONE-DCO-1.0-Signed-off-by: Jonghwa Lee <jonghwa3.lee@samsung.com>
batcheu
force-pushed
the
prevent_resource_leak
branch
from
eacffc0 to
6165656
Compare
| void TearDown() override {} | ||
|
|
||
| std::unique_ptr<BulkPipelineManager> manager; | ||
| const int nr_models = 1; |
There was a problem hiding this comment.
I missed to update this line in previous PR ;(
-- const int nr_models = 2;
++ const int nr_models = 1;
@ragmani |
3 participants
These changes ensure that pipeline resources are properly released even in error scenarios or when the pipeline was not fully initialized, preventing potential resource leaks.
ONE-DCO-1.0-Signed-off-by: Jonghwa Lee jonghwa3.lee@samsung.com