> On Jul 28, 2019, at 11:28 PM, Philipp Kunz <philipp.k...@paratix.ch> wrote:
>
> Hi Max,
>
> jarsignerProc is definitively a spot where significant amount of time is
> spent in the test unlike what is before and what is after it. Have you
> measured how much your change would make the test faster? My bet is it would
> not be significant. Tell me if I'm wrong.
There is some difference. I first applied your recent patch, then my
no-native-library.conf patch and run that test 40 times: mean duration is 153
seconds. Then I apply the change in this mail, it's 59 seconds.
> As I see it, we are, at least to some extent, back to a discussion we already
> started.
Not exactly. Please note that I haven't called "jarsigner -verify" at all, no
matter inside the VM or as a separate process. I simply look into the
CodeSigner (and you have already obtained them) to check the number and names.
> ...
> We might consider removing the parallel=true or add a threadPoolSize
> parameter to it with a suitable value that fits the test machines.
Turns out parallel=true is irrelevant with the failure of this test. It's all
about the native library. And after the disabling of native libraries, even
running jarsigner as a proc is now acceptable.
This makes the fix of this bug non urgent. I am thinking of fixing it in JDK
14, which means I can include your other src changes.
Thanks,
Max
>
> Regards,
> Philipp
>
>
>
> On Thu, 2019-07-25 at 17:38 +0800, Weijun Wang wrote:
>> Hi Philipp,
>>
>> Most are fine but PreserveRawManifestEntryAndDigest.java is still very slow
>> and fails intermittently.
>>
>> The problem is due to jarsignerProc(). Since you have parallel=true for the
>> test providers, many jarsigner processes would run at the same time and some
>> of our test machines cannot load so many processes (they are also running
>> other jobs).
>>
>> I read about the place when jarsignerProc() is called and seems you want to
>> make sure the first file is signed by A and the second is not. Can we do
>> this using JarFile API? Does you have other things to check in
>> getExpectedJarSignerOutputUpdatedContentNotValidatedBySignerA()?
>>
>> How about we remove the `jarsigner -verify` call? Like this:
>>
>> // double-check reading the files with a verifying JarFile
>> try (JarFile jar = new JarFile(jarFilename4, true)) {
>> if (firstAddedFilename != null) {
>> JarEntry je1 = jar.getJarEntry(firstAddedFilename);
>> jar.getInputStream(je1).readAllBytes();
>> - assertTrue(je1.getCodeSigners().length > 0);
>> + CodeSigner[] ss = je1.getCodeSigners();
>> + assertTrue(ss.length == 2);
>> + assertTrue(containsSignerA(ss));
>> }
>> if (secondAddedFilename != null) {
>> JarEntry je2 = jar.getJarEntry(secondAddedFilename);
>> jar.getInputStream(je2).readAllBytes();
>> - assertTrue(je2.getCodeSigners().length > 0);
>> + CodeSigner[] ss = je2.getCodeSigners();
>> + assertTrue(ss.length == 1);
>> + assertFalse(containsSignerA(ss));
>> }
>> }
>>
>> @@ -392,6 +380,17 @@
>> }
>> }
>>
>> + private static boolean containsSignerA(CodeSigner[] ss) {
>> + for (CodeSigner s : ss) {
>> + X509Certificate x = (X509Certificate)
>> + s.getSignerCertPath().getCertificates().get(0);
>> + if (x.getSubjectX500Principal().toString().equals("CN=A")) {
>> + return true;
>> + }
>> + }
>> + return false;
>> + }
>> +
>>
>> Thanks,
>> Max
>>
>>> On Jul 22, 2019, at 10:02 AM, Weijun Wang <weijun.w...@oracle.com> wrote:
>>>
>>> Please take a review at
>>>
>>> http://cr.openjdk.java.net/~weijun/8228456/webrev.00/
>>>
>>> The change is contributed by Philipp Kunz. Since we are now in RDP 2 and we
>>> are not allowed to fix non-test P3 (and lower) bugs. I've removed all src/
>>> changes in Philipp's patch (If I read correctly, is mostly on renaming
>>> methods, and brings no behavior change) and made necessary change in test
>>> codes to use original method names.
>>>
>>> I haven't read the changes yet. I only made sure they pass on my own
>>> system. Oracle's test farm is in maintenance during the weekend.
>>>
>>> Thanks,
>>> Max
>>>
>>
