<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hi Martin,</p>
<p>Thanks for covering additional scenarios.</p>
<p>The updated webrev looks good to me. I will sponsor your patch.</p>
Thanks,<br>
Valerie<br>
<br>
<div class="moz-cite-prefix">On 5/24/2018 12:13 PM, Martin Balao
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAKZz+geW2bGCKZX34uv41Wm8EhVcxxsmmQhMhEsOnyp6quR2=Q@mail.gmail.com">
<div dir="ltr">
<div>Hi Valerie,</div>
<div><br>
</div>
<div>Thanks for your review.</div>
<div><br>
</div>
<div>These are the cases I identified:</div>
<div><br>
</div>
<div> * engineSign and engineVerify include a try-finally block
that releases the session under any circumstances.</div>
<div><br>
</div>
<div> * engineUpdate may leak. Session release added if failure
occurs.</div>
<div><br>
</div>
<div> * cancelOperation may leak. If session has no objects,
it's killed. If it has, a "cancel operation by finishing it"
code is executed. This code can fail on sign/verify
operations, and a session may be leaked. It's not clear to me
why this "cancel operation by finishing it" code is needed,
though. Added a try-finally block anyways.</div>
<div><br>
</div>
<div>Webrev 01:</div>
<div><br>
</div>
<div> * <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.01/"
moz-do-not-send="true">http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.01/</a></div>
<div> * <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.01.zip"
moz-do-not-send="true">http://cr.openjdk.java.net/~mbalao/webrevs/8203182/8203182.webrev.01.zip</a></div>
<div><br>
</div>
<div>Look forward to your answer.</div>
<div><br>
</div>
<div>Kind regards,</div>
<div>Martin.-</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Tue, May 15, 2018 at 7:30 PM,
Valerie Peng <span dir="ltr"><<a
href="mailto:valerie.peng@oracle.com" target="_blank"
moz-do-not-send="true">valerie.peng@oracle.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Martin,<br>
<br>
Your fix only addresses the initialization case. Have you
considered fixing the same problem under difference calls?<br>
<br>
Regards,<br>
Valerie<span class=""><br>
<br>
On 5/14/2018 3:25 PM, Martin Balao wrote:<br>
</span>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"><span
class=""> Hi,<br>
<br>
Can I have a review for "JDK-8203182 - Release session
if initialization of SunPKCS11 Signature fails" [1]?<br>
<br>
</span> * <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~mb<wbr>alao/webrevs/8203182/8203182.<wbr>webrev.00/</a>
<<a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00/"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7<wbr>Embalao/webrevs/8203182/820318<wbr>2.webrev.00/</a>><br>
* <a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00.zip"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~mb<wbr>alao/webrevs/8203182/8203182.<wbr>webrev.00.zip</a>
<<a
href="http://cr.openjdk.java.net/%7Embalao/webrevs/8203182/8203182.webrev.00.zip"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/%7<wbr>Embalao/webrevs/8203182/820318<wbr>2.webrev.00.zip</a>><span
class=""><br>
<br>
Thanks in advanced!<br>
<br>
Martin.-<br>
<br>
--<br>
[1] - <a
href="https://bugs.openjdk.java.net/browse/JDK-8203182"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://bugs.openjdk.java.net/<wbr>browse/JDK-8203182</a><br>
</span></blockquote>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>