RFR: 8238196: tests that use SA Attach should not be allowed to run against signed binaries on Mac OS X 10.14.5 and later
Chris Plummer
chris.plummer at oracle.com
Wed Feb 12 23:48:59 UTC 2020
On 2/12/20 2:15 PM, David Holmes wrote:
> On 13/02/2020 5:02 am, Chris Plummer wrote:
>> Hi David,
>>
>>> What you have below is a mix of #2 and #3 - you convert to a generic
>>> exception but also re-assert the interrupt state. That's a little
>>> unusual.
>> That what I also thought which is why I was suggesting not doing the
>> interrupt() call and only throwing the RuntimeException. I agree that
>> doing both does not make sense, and in general doing (3) does not
>> make sense if the caller is not setup to properly check the interrupt
>> state.
>
> From a library writer perspective you should have zero knowledge of
> the caller and doing (3) makes perfect sense. Remember that you will
> only re-assert the interrupt() if you get the InterruptedException in
> the first place, which means that some other code already issued the
> initial interrupt().
>
> Whether this code shoud be treated as general purpose library code is
> another matter.
>
> Personally, in this case I think I'd use (2) and make interruption a
> failure mode.
Ok. We're in agreement here. Igor, are you ok with this?
try {
...
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
thanks,
Chris
>
> David
>
>> Chris
>>
>>
>> On 2/12/20 5:58 AM, David Holmes wrote:
>>> Hi Chris,
>>>
>>> I think you are overthinking this. :)
>>>
>>> What you have observed is that the code that actually uses this
>>> method does not utilise interrupts, or expect them, so if you
>>> artifically inject one in this library method then you see things
>>> failing in unexpected ways. That also means that if the thread was
>>> interrupted by some other piece of logic then it would also fail in
>>> unexpected ways. That doesn't negate your choice to re-assert the
>>> interrupt state.
>>>
>>> From a library writing perspective if you have a method that
>>> performs a blocking call that can throw InterruptedException then
>>> you generally have three choices:
>>>
>>> 1. Throw InterruptedException yourself and pass the buck to your
>>> callers.
>>> 2. Convert the InterruptedException to a more general failure
>>> exception - typically an unchecked RuntimeException - for which
>>> interruption is but one possible cause; or
>>> 3. Catch the InterruptedException and allow the method to complete
>>> normally (i.e. not by throwing an exception) but re-assert the
>>> interrupt state so that a caller checking for interruption will
>>> still see that it occurred.
>>>
>>> What you have below is a mix of #2 and #3 - you convert to a generic
>>> exception but also re-assert the interrupt state. That's a little
>>> unusual.
>>>
>>> David
>>>
>>>
>>> On 12/02/2020 6:16 pm, Chris Plummer wrote:
>>>> Hi Igor,
>>>>
>>>> I think it might be best to the interrupt() call out. I wanted to
>>>> see what would happen if we ever got an InterruptedException, so I
>>>> added the following to the start of Platform.shouldSAAttach():
>>>>
>>>> try {
>>>> throw new InterruptedException();
>>>> } catch (InterruptedException e) {
>>>> Thread.currentThread().interrupt();
>>>> throw new RuntimeException(e);
>>>> }
>>>>
>>>> At the start of the test run, before any tests are actually run, I
>>>> see the following:
>>>>
>>>> failed to get value for vm.hasSAandCanAttach
>>>> java.lang.RuntimeException: java.lang.InterruptedException
>>>> at jdk.test.lib.Platform.shouldSAAttach(Platform.java:300)
>>>> at requires.VMProps.vmHasSAandCanAttach(VMProps.java:327)
>>>> at requires.VMProps$SafeMap.put(VMProps.java:69)
>>>> at requires.VMProps.call(VMProps.java:101)
>>>> at requires.VMProps.call(VMProps.java:57)
>>>> at
>>>> com.sun.javatest.regtest.agent.GetJDKProperties.run(GetJDKProperties.java:80)
>>>>
>>>> at
>>>> com.sun.javatest.regtest.agent.GetJDKProperties.main(GetJDKProperties.java:54)
>>>>
>>>> Caused by: java.lang.InterruptedException
>>>> at jdk.test.lib.Platform.shouldSAAttach(Platform.java:297)
>>>> ... 6 more
>>>>
>>>> This seems reasonable.
>>>>
>>>> For each test that checks vm.hasSAandCanAttach I also see.
>>>>
>>>> TEST RESULT: Error. Error evaluating expression:
>>>> vm.hasSAandCanAttach: java.lang.RuntimeException:
>>>> java.lang.InterruptedException
>>>>
>>>> This too seems reasonable.
>>>>
>>>> For tests that don't check vm.hasSAandCanAttach, but instead make a
>>>> runtime check that calls Platform.shouldSAAttach(), the test fails
>>>> with:
>>>>
>>>> java.lang.IllegalThreadStateException: process hasn't exited
>>>> at
>>>> java.base/java.lang.ProcessImpl.exitValue(ProcessImpl.java:500)
>>>> at jdk.test.lib.apps.LingeredApp.stopApp(LingeredApp.java:380)
>>>> at jdk.test.lib.apps.LingeredApp.stopApp(LingeredApp.java:433)
>>>> at ClhsdbAttach.main(ClhsdbAttach.java:77)
>>>> at
>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>> Method)
>>>> at
>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>
>>>> at
>>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>
>>>> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
>>>> at
>>>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>>>>
>>>> at java.base/java.lang.Thread.run(Thread.java:832)
>>>>
>>>> This is a confusing way to fail. The reason it fails this way is
>>>> because stopApp() first calls waitAppTerminiate(), which does the
>>>> following:
>>>>
>>>> public void waitAppTerminate() {
>>>> // This code is modeled after tail end of
>>>> ProcessTools.getOutput().
>>>> try {
>>>> appProcess.waitFor();
>>>> outPumperThread.join();
>>>> errPumperThread.join();
>>>> } catch (InterruptedException e) {
>>>> Thread.currentThread().interrupt();
>>>> // pass
>>>> }
>>>> }
>>>>
>>>> I added an e.printStackTrace() call and see the following:
>>>>
>>>> java.lang.InterruptedException
>>>> at java.base/java.lang.Object.wait(Native Method)
>>>> at java.base/java.lang.Object.wait(Object.java:321)
>>>> at java.base/java.lang.ProcessImpl.waitFor(ProcessImpl.java:474)
>>>> at
>>>> jdk.test.lib.apps.LingeredApp.waitAppTerminate(LingeredApp.java:239)
>>>> at jdk.test.lib.apps.LingeredApp.stopApp(LingeredApp.java:380)
>>>> at jdk.test.lib.apps.LingeredApp.stopApp(LingeredApp.java:434)
>>>>
>>>> So the earlier call to interrupt() is resulting in
>>>> waitAppTerminate() not actually waiting for exit. This then results
>>>> in stopApp() getting IllegalThreadStateException when calling
>>>> Process.exitValue().
>>>>
>>>> If I comment out the call to interrupt() in
>>>> Platform.shouldSAAttach(), I think the failure stack trace is much
>>>> better:
>>>>
>>>> java.lang.RuntimeException: Test ERROR java.lang.RuntimeException:
>>>> java.lang.InterruptedException
>>>> at ClhsdbAttach.main(ClhsdbAttach.java:75)
>>>> at
>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
>>>> Method)
>>>> at
>>>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
>>>>
>>>> at
>>>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>>>
>>>> at java.base/java.lang.reflect.Method.invoke(Method.java:564)
>>>> at
>>>> com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
>>>>
>>>> at java.base/java.lang.Thread.run(Thread.java:832)
>>>> Caused by: java.lang.RuntimeException: java.lang.InterruptedException
>>>> at jdk.test.lib.Platform.shouldSAAttach(Platform.java:300)
>>>> at ClhsdbLauncher.run(ClhsdbLauncher.java:199)
>>>> at ClhsdbAttach.main(ClhsdbAttach.java:71)
>>>> ... 6 more
>>>> Caused by: java.lang.InterruptedException
>>>> at jdk.test.lib.Platform.shouldSAAttach(Platform.java:297)
>>>> ... 8 more
>>>>
>>>> There's still a minor issue with rethrowing the RuntimeException
>>>> encapsulated inside another RuntimeException. That the fault of the
>>>> test which is catching all Exceptions and encapsulating them in a
>>>> RuntimeException, even if the Exceptions itself is already a
>>>> RuntimeException. It should add have a catch clause for
>>>> RuntimeException, and just rethrow it without encapulating it. All
>>>> the Clhsdb tests seem to do this, so that's about 20 places to fix.
>>>> Probably not worth doing unless some other cleanup is being done at
>>>> the same time.
>>>>
>>>> Chris
>>>>
>>>> On 2/11/20 10:30 PM, Igor Ignatyev wrote:
>>>>> I'd say yes, it's better to still call Thread::interrupt.
>>>>>
>>>>> -- Igor
>>>>>
>>>>>> On Feb 11, 2020, at 10:19 PM, Chris Plummer
>>>>>> <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>> wrote:
>>>>>>
>>>>>> Ok. Should I still call interrupt()?
>>>>>>
>>>>>> Chris
>>>>>>
>>>>>> On 2/11/20 10:07 PM, Igor Ignatyev wrote:
>>>>>>> Hi Chris,
>>>>>>>
>>>>>>> that's a common practice for any kind of library-ish code, if
>>>>>>> there are no explicit check of interrupt status, it will be
>>>>>>> checked a by next operation which might be interrupted. in this
>>>>>>> particular case, I agree rethrowing it as an unchecked exception
>>>>>>> might be a good alternative.
>>>>>>>
>>>>>>> -- Igor
>>>>>>>
>>>>>>>> On Feb 11, 2020, at 10:03 PM, Chris Plummer
>>>>>>>> <chris.plummer at oracle.com <mailto:chris.plummer at oracle.com>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>> Hi Igor,
>>>>>>>>
>>>>>>>> I guess I fail to see the benefit of this. Who is going to
>>>>>>>> check the interrupt status of this thread and do something
>>>>>>>> meaningful with it? It seems we would want to immediately
>>>>>>>> propagate the failure by throwing a RuntimeException. This will
>>>>>>>> work well when called from a test since this is a common way to
>>>>>>>> fail a test. The other use of this code is by
>>>>>>>> VMProps.vmHasSAandCanAttach(). It looks like if a
>>>>>>>> RuntimeException is thrown the right thing will happen when
>>>>>>>> SafeMap.put() catches the exception (it catches all Throwables).
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>> On 2/11/20 7:12 PM, Igor Ignatev wrote:
>>>>>>>>> rather like this :
>>>>>>>>>
>>>>>>>>>> } catch (InterruptedException e) {
>>>>>>>>>> Thread.currentThread().interrupt();
>>>>>>>>>> return false; // assume not signed
>>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> — Igor
>>>>>>>>>
>>>>>>>>>> On Feb 11, 2020, at 6:15 PM, Chris Plummer
>>>>>>>>>> <Chris.Plummer at oracle.com <mailto:Chris.Plummer at oracle.com>>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Like this?
>>>>>>>>>>
>>>>>>>>>> } catch (InterruptedException e) {
>>>>>>>>>> Thread.currentThread().interrupt();
>>>>>>>>>> throw new RuntimeException(e);
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> Chris
>>>>>>>>>>
>>>>>>>>>> On 2/11/20 2:23 PM, Igor Ignatyev wrote:
>>>>>>>>>>> no, I meant to call Thread.currentThread().interrupt(),
>>>>>>>>>>> calling that will restore interrupted state of the thread,
>>>>>>>>>>> so an user of Platform class will be able to response to it
>>>>>>>>>>> appropriately, w/ your current code, the fact that the
>>>>>>>>>>> thread was interrupted will be missed, and in most cases it
>>>>>>>>>>> is not right thing to do.
>>>>>>>>>>>
>>>>>>>>>>> -- Igor
>>>>>>>>>>>
>>>>>>>>>>>> On Feb 11, 2020, at 2:02 PM, Chris Plummer
>>>>>>>>>>>> <chris.plummer at oracle.com
>>>>>>>>>>>> <mailto:chris.plummer at oracle.com>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Igor,
>>>>>>>>>>>>
>>>>>>>>>>>> I'm not sure what you mean by restore the interrupt state.
>>>>>>>>>>>> Do you mean loop back to the waitFor() call?
>>>>>>>>>>>>
>>>>>>>>>>>> thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> Chris
>>>>>>>>>>>>
>>>>>>>>>>>> On 2/11/20 1:55 PM, Igor Ignatyev wrote:
>>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I don't insist on (3), so I'm fine if you don't want to
>>>>>>>>>>>>> change that part. one thing I'd change though is to
>>>>>>>>>>>>> restore thread interrupted state at L#266 of Platform.java
>>>>>>>>>>>>> (no need to publish new webrev)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> -- Igor
>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Feb 11, 2020, at 1:49 PM, Chris Plummer
>>>>>>>>>>>>>> <chris.plummer at oracle.com
>>>>>>>>>>>>>> <mailto:chris.plummer at oracle.com>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Igor,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Here's an updated webrev:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238196/webrev.01/index.html
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I rebased to JDK 15 and made all the changes you
>>>>>>>>>>>>>> suggested except for (3). I did not think it is necessary
>>>>>>>>>>>>>> since the code is only executed on OSX. However, if you
>>>>>>>>>>>>>> still feel allowing flexibility in the path separator is
>>>>>>>>>>>>>> important, I can add that change too.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 2/10/20 1:34 PM, Igor Ignatyev wrote:
>>>>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> in general it all looks good, I have a few comments
>>>>>>>>>>>>>>> (most of them are editorial):
>>>>>>>>>>>>>>> in Platform.java:
>>>>>>>>>>>>>>> 1. you have doubled spaced at line#238 (b/w boolean
>>>>>>>>>>>>>>> and isSignedOSX)
>>>>>>>>>>>>>>> 2. as FileNotFoundException is IOException, there is no
>>>>>>>>>>>>>>> need to declare the former in the signature of isSignedOSX
>>>>>>>>>>>>>>> 3. it's better to pass jdkPath, "bin" and "java" as
>>>>>>>>>>>>>>> separate arguments to Path.get, so the code won't depend
>>>>>>>>>>>>>>> on file separator
>>>>>>>>>>>>>>> 4. you are waiting for codesign to finish w/o reading
>>>>>>>>>>>>>>> its cout / cerr, which might lead to a deadlock
>>>>>>>>>>>>>>> (if codesign will exhaust IO buffer before exiting), so
>>>>>>>>>>>>>>> you need to either create two separate threads to read
>>>>>>>>>>>>>>> cout and cerr or redirect these streams them to files
>>>>>>>>>>>>>>> and read these files afterwards or just ignore cout/cerr
>>>>>>>>>>>>>>> by using Redirect.DISCARD. I'd personally recommend the
>>>>>>>>>>>>>>> latter as the result of codesign can be reliably deduced
>>>>>>>>>>>>>>> from its exitcode (0 - signed, 1 - verification failed,
>>>>>>>>>>>>>>> 2 - wrong arguments, 3 - not all requirements from R are
>>>>>>>>>>>>>>> satisfied) and using cout/cerr is somewhat fragile as
>>>>>>>>>>>>>>> there is no guarantee output format won't be changed.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> the rest looks good to me.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -- Igor
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Feb 10, 2020, at 11:48 AM, Chris Plummer
>>>>>>>>>>>>>>>> <chris.plummer at oracle.com
>>>>>>>>>>>>>>>> <mailto:chris.plummer at oracle.com><mailto:chris.plummer at oracle.com>>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Ping #2. It's not that hard of a review. Most of it is
>>>>>>>>>>>>>>>> the new Platform.isSignedOSX() method, which is well
>>>>>>>>>>>>>>>> commented and pretty straight froward.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 2/4/20 5:04 PM, Chris Plummer wrote:
>>>>>>>>>>>>>>>>> Ping!
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> And I decided to push to 15 instead of 14. Will
>>>>>>>>>>>>>>>>> backport to 14 eventually.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On 1/30/20 10:20 PM, Chris Plummer wrote:
>>>>>>>>>>>>>>>>>> Yes, you are correct:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8238196
>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8238196/webrev.00
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On 1/30/20 10:13 PM, Igor Ignatyev wrote:
>>>>>>>>>>>>>>>>>>> Hi Chris,
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8236913/webrev.00Â
>>>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~cjplummer/8236913/webrev.00%C3%82> seems
>>>>>>>>>>>>>>>>>>> to be a webrev from another issue, should it have
>>>>>>>>>>>>>>>>>>> beenÂhttp://cr.openjdk.java.net/~cjplummer/8238196/webrev.00/Â
>>>>>>>>>>>>>>>>>>> <http://cr.openjdk.java.net/~cjplummer/8238196/webrev.00/%C3%82> ?
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> -- Igor
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On Jan 30, 2020, at 10:10 PM, Chris Plummer
>>>>>>>>>>>>>>>>>>>> <chris.plummer at oracle.com
>>>>>>>>>>>>>>>>>>>> <mailto:chris.plummer at oracle.com><mailto:chris.plummer at oracle.com>>
>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Please review the following fix for some SA tests
>>>>>>>>>>>>>>>>>>>> that are failing on Mac OS X 10.14.5 and later:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8238196
>>>>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8236913/webrev.00
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> The issue is that SA can't attach to a signed
>>>>>>>>>>>>>>>>>>>> binary starting with 10.14.5. There is no
>>>>>>>>>>>>>>>>>>>> workaround for this, so these tests are being
>>>>>>>>>>>>>>>>>>>> disabled when it is detected that the binary is
>>>>>>>>>>>>>>>>>>>> signed and we are running on 10.14 or later (I
>>>>>>>>>>>>>>>>>>>> chose all 10.14 releases to simplify the check).
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Some background may help explain the fix. In order
>>>>>>>>>>>>>>>>>>>> for SA to attach to a live process (not a core
>>>>>>>>>>>>>>>>>>>> file) on OSX, either the attaching process (ie. the
>>>>>>>>>>>>>>>>>>>> test) has to be run as root, or sudo needs to be
>>>>>>>>>>>>>>>>>>>> supported. However, the only tests that make the
>>>>>>>>>>>>>>>>>>>> sudo check are the 20 or so that use
>>>>>>>>>>>>>>>>>>>> ClhsdbLauncher. The rest all rely on "@requires
>>>>>>>>>>>>>>>>>>>> vm.hasSAandCanAttach" to filter out tests that use
>>>>>>>>>>>>>>>>>>>> SA attach. vm.hasSAandCanAttach only checks if the
>>>>>>>>>>>>>>>>>>>> test is being run as root. Thus all our
>>>>>>>>>>>>>>>>>>>> non-ClhsdbLauncher tests that SA attach to a live
>>>>>>>>>>>>>>>>>>>> process are currently not run unless they are run
>>>>>>>>>>>>>>>>>>>> as root. 8238268 [1] has been filed to address
>>>>>>>>>>>>>>>>>>>> this, making it so all the tests will attempt to
>>>>>>>>>>>>>>>>>>>> use sudo if not run as root.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Because of the difference in how ClhsdbLauncher
>>>>>>>>>>>>>>>>>>>> tests and "@requires vm.hasSAandCanAttach" tests
>>>>>>>>>>>>>>>>>>>> check to see if they are runnable, this fix needs
>>>>>>>>>>>>>>>>>>>> to address both types of checks. The common code
>>>>>>>>>>>>>>>>>>>> for both these cases is Platform.shouldSAAttach(),
>>>>>>>>>>>>>>>>>>>> which on OSX basically equates to check to see if
>>>>>>>>>>>>>>>>>>>> we are running as root. I changed it to also return
>>>>>>>>>>>>>>>>>>>> false if running on signed binary with 10.14 or
>>>>>>>>>>>>>>>>>>>> later. However, this confused the ClhsdbLauncher
>>>>>>>>>>>>>>>>>>>> use of Platform.shouldSAAttach() somewhat, since it
>>>>>>>>>>>>>>>>>>>> assumed a false result only happens because you are
>>>>>>>>>>>>>>>>>>>> not running as root (in which case it would then
>>>>>>>>>>>>>>>>>>>> check if sudo will work). So ClhsdbLauncher now has
>>>>>>>>>>>>>>>>>>>> double check that the false result was not because
>>>>>>>>>>>>>>>>>>>> of running a signed binary. If it is signed, it
>>>>>>>>>>>>>>>>>>>> won't do the sudo check. This will get cleaned up
>>>>>>>>>>>>>>>>>>>> with 8238268 [1], which will move the sudo check
>>>>>>>>>>>>>>>>>>>> into Platform.shouldSAAttach().
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> thanks,
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Chris
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8238268
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>>
More information about the serviceability-dev
mailing list