Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] fix thread safe issue in #19832 #24562

Merged

Conversation

tiger100256-hu
Copy link
Contributor

@tiger100256-hu tiger100256-hu commented May 17, 2024

Details:

Tickets:

I lost code "t_stream_count_map[(void*)this] = item.first;" in openvinotoolkit#19832
the thread safe issue happen in below workflow
create thread A
call CustomThreadLocal:local() in thread A -> create stream A (the count of stream A is 2)
destory thread A (the count of stream A is 1)
create thread B (same thread id with thread A)
call CustomThreadLocal:local() in thread B -> use stream A(the count of stream A is 1, so it's broken)

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
@github-actions github-actions bot added the category: inference OpenVINO Runtime library - Inference label May 17, 2024
@ilya-lavrenov ilya-lavrenov added bug Something isn't working pr: needs tests PR needs tests updating labels May 19, 2024
Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
@tiger100256-hu tiger100256-hu marked this pull request as ready for review May 21, 2024 09:07
@tiger100256-hu tiger100256-hu requested review from a team as code owners May 21, 2024 09:07
Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
@riverlijunjie
Copy link
Contributor

Can we add test cases for 2 models to concurrently create infer and do infer, including sync infer and async infer?

@tiger100256-hu
Copy link
Contributor Author

@riverlijunjie ok, I will update the test case

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
@tiger100256-hu
Copy link
Contributor Author

@riverlijunjie already add test case for two model and async infer, please help to continue review

Copy link
Contributor

@riverlijunjie riverlijunjie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

tests/stress_tests/common/ie_pipelines/pipelines.cpp Outdated Show resolved Hide resolved
@ilya-lavrenov ilya-lavrenov removed the pr: needs tests PR needs tests updating label May 23, 2024
use space instead of tab

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
@ilya-lavrenov ilya-lavrenov added this to the 2024.2 milestone May 23, 2024
@wangleis wangleis enabled auto-merge May 23, 2024 07:54
@ilya-lavrenov ilya-lavrenov added the port to 2023.3 Need port from master to 2023.3 LTS label May 23, 2024
@wangleis wangleis added this pull request to the merge queue May 23, 2024
Merged via the queue into openvinotoolkit:master with commit 1a4ae5e May 23, 2024
116 of 117 checks passed
tiger100256-hu added a commit to tiger100256-hu/openvino that referenced this pull request May 24, 2024
…t#24562)

- *I lost code `t_stream_count_map[(void*)this] = item.first;` in
 - *the thread safe issue happen in below workflow*
 - create thread A
call CustomThreadLocal:local() in thread A -> create stream A (the count
of stream A is 2)
   destory thread A (the count of stream A is 1)
   create thread B (same thread id with thread A)
call CustomThreadLocal:local() in thread B -> use stream A(the count of
stream A is 1, so it's broken)
- *add testcase, also fix
https://github.com/openvinotoolkit/openvino/pull/19986/files#r1332774754*

 - Closes openvinotoolkit#24509

---------

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
Co-authored-by: Wanglei Shen <wanglei.shen@intel.com>
@ilya-lavrenov ilya-lavrenov removed the port to 2023.3 Need port from master to 2023.3 LTS label May 27, 2024
wangleis added a commit that referenced this pull request May 27, 2024
### Details:
 - *port PR #24562 to release 2023.3*
 - *add support for InferAPI1 and api_version*

### Tickets:
-
*[issue-24509](#24509

---------

Signed-off-by: HU Yuan2 <yuan2.hu@intel.com>
Co-authored-by: Wanglei Shen <wanglei.shen@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working category: inference OpenVINO Runtime library - Inference Code Freeze no-match-files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: ov::InferRequest::infer() not thread safe when having multiple models
5 participants