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

Fixes cases where the getEngine method in the EngineProvider class returns null when called concurrently. #3005

Merged
merged 2 commits into from
Feb 26, 2024

Conversation

onaple
Copy link
Contributor

@onaple onaple commented Feb 26, 2024

Description

When I create models concurrently using the following methods, I always encounter a null pointer exception because of a problem with the Engine.getEngine("PyTorch ") double-check lock. I wonder if the author has any other intention in this way. However, it is necessary for users to deal with outer layer exceptions.

Model localModel = Engine.getEngine("PyTorch").newModel("test", Device.cpu())

@onaple onaple requested review from zachgk, frankfliu and a team as code owners February 26, 2024 03:01
@frankfliu
Copy link
Contributor

@onaple
Thanks for report this issue. This bug was introduced by this PR: #2885

I think we should just revert #2885 completely.

@codecov-commenter
Copy link

codecov-commenter commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 79.31034% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 72.21%. Comparing base (7959c27) to head (c81de78).

Files Patch % Lines
api/src/main/java/ai/djl/engine/Engine.java 69.23% 2 Missing and 2 partials ⚠️
...n/java/ai/djl/pytorch/engine/PtEngineProvider.java 50.00% 0 Missing and 1 partial ⚠️
...ava/ai/djl/tensorflow/engine/TfEngineProvider.java 50.00% 0 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3005      +/-   ##
============================================
- Coverage     72.29%   72.21%   -0.09%     
+ Complexity     7292     7280      -12     
============================================
  Files           722      722              
  Lines         32525    32483      -42     
  Branches       3395     3388       -7     
============================================
- Hits          23515    23458      -57     
- Misses         7385     7412      +27     
+ Partials       1625     1613      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@frankfliu
Copy link
Contributor

@onaple

DJL 0.25.0 doesn't have this issue. You can use 0.25.0 for the mean time.

@frankfliu frankfliu merged commit 19e4302 into deepjavalibrary:master Feb 26, 2024
5 checks passed
frankfliu added a commit that referenced this pull request Apr 26, 2024
…turns null when called concurrently. (#3005)

* Fixes cases where the getEngine method in the EngineProvider class returns null when called concurrently.

* Revert "Creates DJL manual engine initialization (#2885)"

This reverts commit 6141c48.

---------

Co-authored-by: 王旭 <wangxu75@meituan.com>
Co-authored-by: Frank Liu <frankfliu2000@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants