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

8252412: [macos11] system dynamic libraries removed from filesystem #2119

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter
Filter file types
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -110,8 +110,26 @@ private static String getLibraryName() throws IOException {
// if LIB2 exists, use that
return lib;
}

// As of macos 11, framework libraries have been removed from the file
// system, but in such a way that they can still be dlopen()ed, even
// though they can no longer be open()ed.
//
// https://developer.apple.com/documentation/macos-release-notes/macos-big-sur-11_0_1-release-notes
//
// """New in macOS Big Sur 11.0.1, the system ships with a built-in
// dynamic linker cache of all system-provided libraries. As part of
// this change, copies of dynamic libraries are no longer present on
// the filesystem. Code that attempts to check for dynamic library
// presence by looking for a file at a path or enumerating a directory
// will fail. Instead, check for library presence by attempting to
// dlopen() the path, which will correctly check for the library in the
// cache."""
//
// The directory structure remains otherwise intact, so check for
// existence of the containing directory instead of the file.
lib = PCSC_FRAMEWORK;
if (new File(lib).isFile()) {
if (new File(lib).getParentFile().isDirectory()) {

This comment has been minimized.

@jianglizhou

jianglizhou Jan 22, 2021
Contributor

This is outside of my normal area, could you please explain why checking the existence of the containing directory is equivalent to checking the file here? Does it provide the expected behavior on all unix-like platforms or only macos?

Could you please also provide a jtreg test for this change?

This comment has been minimized.

@Martin-Buchholz

Martin-Buchholz Jan 22, 2021
Author Member

The directory structure is intact - only the file is removed from the filesystem.
More generally, for many frameworks, where there used to be

/System/Library/Frameworks//.framework/Versions/Current/

the file is gone, but

/System/Library/Frameworks//.framework/Versions/Current/

remains.

I don't think we need a jtreg test, since any functionality associated with PCSC is broken on this platform. I added label noreg-other

This comment has been minimized.

@jianglizhou

jianglizhou Jan 22, 2021
Contributor

Ok, I see Java_sun_security_smartcardio_PlatformPCSC_initialize does dlopen using the 'jLibName' (string) obtained from getLibraryName() and throws IOException if dlopen fails. The change seems safe enough.

I'm wondering if you want to check the file first then check the parent directory if the file does not exist. Not sure if that's a little more optimal on older macos, so I'll leave that to you to decide.

For the jtreg test, how about converting Dominik's TestPCSC? As the file is a shared for 'unix' platforms, it feels safer at least with some level of unit test. Could you please give some more contexts about the functionalities associated with PCSC are broken on macos?

This comment has been minimized.

@jianglizhou

jianglizhou Jan 22, 2021
Contributor

Martin and I had an off-line chat and Martin convinced me that the existing jtreg tests (such as test/jdk/javax/smartcardio and test/jdk/sun/security/smartcardio are sufficient) to cover the case.

This comment has been minimized.

@valeriepeng

valeriepeng Jan 23, 2021

Right, existing tests should cover this already since running the test requires that the library must be loaded.
Changes look fine, thanks for fixing this.
Kind of surprised the existing filtering didn't catch this as security-related changes and send this to security group for review.

// if PCSC.framework exists, use that
return lib;
}
ProTip! Use n and p to navigate between commits in a pull request.