Fix segfault for decoupled models #327

krishung5 · 2023-12-12T02:15:02Z

This PR fixed the segfault issues:

when an exception is raised within a decoupled model. We expected users to handle error by returning an error response with a final flag like this https://github.com/triton-inference-server/server/blob/main/qa/L0_backend_python/decoupled/models/decoupled_execute_error/1/model.py#L95-L106. This fix should also cover the use case where an exception is raised by calling raise Exception("Intentional Error") without sending any final flag via response sender.
when there are lots of concurrent requests to decoupled models, the request object may have the same address which causes issues when using that address as id to retrieve the response factory from the map. This PR removed the usage of response_factory_map. The backend will create a new response factory for each request, no matter if the request is a rescheduled one or not, and clean up the response_factory when response_sender goes out of scope in the Python model to avoid memory growth.

Added testing: triton-inference-server/server#6686

…error

src/python_be.cc

Tabrizian

Is it going to be cherry-picked for 23.12?

krishung5 · 2023-12-13T18:01:58Z

Is it going to be cherry-picked for 23.12?

This is to fix a segfault so I think we could if we have not staged the container. @tanmayv25 May I have your thoughts on this?

src/ipc_message.h

* Set release flags and clean up response factory map before returning error * Address comments * Move the cleanup function to the outside scope * Delete response factory when response sender goes out of scope

Set release flags and clean up response factory map before returning …

fe2cd50

…error

krishung5 requested review from Tabrizian and rmccorm4 December 12, 2023 02:15

krishung5 mentioned this pull request Dec 12, 2023

Add test case for decoupled model raising exception triton-inference-server/server#6686

Merged

rmccorm4 reviewed Dec 12, 2023

View reviewed changes

src/python_be.cc Outdated Show resolved Hide resolved

krishung5 requested a review from rmccorm4 December 12, 2023 02:36

nnshah1 reviewed Dec 12, 2023

View reviewed changes

src/python_be.cc Outdated Show resolved Hide resolved

krishung5 added 2 commits December 12, 2023 11:40

Address comments

419a3a5

Move the cleanup function to the outside scope

f8b2eb6

krishung5 requested a review from nnshah1 December 12, 2023 22:28

Tabrizian approved these changes Dec 13, 2023

View reviewed changes

rmccorm4 approved these changes Dec 13, 2023

View reviewed changes

Delete response factory when response sender goes out of scope

489217f

krishung5 changed the title ~~Set release flags and clean up response factory map before returning …~~ Fix segfault for decoupled models Dec 14, 2023

krishung5 requested a review from Tabrizian December 14, 2023 22:57

Tabrizian reviewed Dec 15, 2023

View reviewed changes

src/ipc_message.h Show resolved Hide resolved

Tabrizian approved these changes Dec 15, 2023

View reviewed changes

krishung5 merged commit c5f304d into main Dec 15, 2023
3 checks passed

krishung5 mentioned this pull request Dec 15, 2023

Update name of ipc message type #329

Merged

krishung5 deleted the krish-decoupled branch May 30, 2024 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix segfault for decoupled models #327

Fix segfault for decoupled models #327

krishung5 commented Dec 12, 2023 •

edited

Loading

Tabrizian left a comment

krishung5 commented Dec 13, 2023 •

edited

Loading

Fix segfault for decoupled models #327

Fix segfault for decoupled models #327

Conversation

krishung5 commented Dec 12, 2023 • edited Loading

Tabrizian left a comment

Choose a reason for hiding this comment

krishung5 commented Dec 13, 2023 • edited Loading

krishung5 commented Dec 12, 2023 •

edited

Loading

krishung5 commented Dec 13, 2023 •

edited

Loading