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

Fix race condition of loadedLibraries #246

Merged
merged 2 commits into from
Jul 28, 2018
Merged

Fix race condition of loadedLibraries #246

merged 2 commits into from
Jul 28, 2018

Conversation

louxiu
Copy link
Contributor

@louxiu louxiu commented Jul 28, 2018

We should put lib to loadedLibraries only after System.load(lib)
success, or loadedLibraries.get(lib) will get it in other thread
before we remove it after System.load(lib) failed.

We should put lib to loadedLibraries only after System.load(lib)
success, or loadedLibraries.get(lib) will get it in other thread
before we remove it after System.load(lib) failed.
@saudet
Copy link
Member

saudet commented Jul 28, 2018

The reason we can't do that is because System.load() calls JNI_OnLoad(), which might start loading other classes that may include the class we're already trying to load, as part of some cycle. We need a way to detect that we're already loading a given class. What issues are you encountering with the current scheme?

@saudet
Copy link
Member

saudet commented Jul 28, 2018

I see, a race condition could occur in there if we're not loading from a static { } block or something. Those are basically synchronized, maybe we should make Loader.loadLibrary() synchronized as well?

@louxiu
Copy link
Contributor Author

louxiu commented Jul 28, 2018

Another confusion about this line. platform.link is

A list of libraries the native compiler should link with

Why it is necessary to try to load them when we run the application(static {Loader.load();})?

@saudet
Copy link
Member

saudet commented Jul 28, 2018

When we link against shared libraries, yes, we need to try to load them. But we can disable that behavior by suffixing them with #, which is used to rename library files on load, but if no name is given, they just don't get loaded.

@saudet saudet merged commit 69d3921 into bytedeco:master Jul 28, 2018
@louxiu louxiu deleted the race_condition branch July 29, 2018 06:29
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.

2 participants