yes, I'm fine w/ that.
-- Igor
> On Feb 12, 2020, at 3:48 PM, Chris Plummer <chris.plum...@oracle.com> wrote:
>
> 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.plum...@oracle.com
>>>>>>> <mailto:chris.plum...@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.plum...@oracle.com
>>>>>>>>> <mailto:chris.plum...@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.plum...@oracle.com <mailto:chris.plum...@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.plum...@oracle.com <mailto:chris.plum...@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.plum...@oracle.com <mailto:chris.plum...@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.plum...@oracle.com
>>>>>>>>>>>>>>>>> <mailto:chris.plum...@oracle.com><mailto:chris.plum...@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.plum...@oracle.com
>>>>>>>>>>>>>>>>>>>>> <mailto:chris.plum...@oracle.com><mailto:chris.plum...@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
