<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; color: rgb(0, 0, 0); font-size: 14px; font-family: Calibri, sans-serif;">
<div>If the API says</div>
<div><br>
</div>
<div>const bool GetInverse(Self *inverse) const;</div>
<div><br>
</div>
<div>If that changes the internal state of the class, isn’t that false advertising?</div>
<div><br>
</div>
<div>Actually, I tried just fixing that one problem in MatrixOffsetTransformBase, and it just kicked the problem slightly down the road.</div>
<div><br>
</div>
<div>ImageMaskSpatialObject::IsInside also advertises as being a const method; yet it calls SetInternalInverseTransformToWorldToIndexTransform() which is ALSO marked as const, and yet modifies an instance variable.</div>
<div><br>
</div>
<div><br>
</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<div style="font-family:Calibri; font-size:11pt; text-align:left; color:black; BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<span style="font-weight:bold">From: </span>Bradley Lowekamp <<a href="mailto:blowekamp@mail.nih.gov">blowekamp@mail.nih.gov</a>><br>
<span style="font-weight:bold">Date: </span>Friday, October 24, 2014 at 12:25 PM<br>
<span style="font-weight:bold">To: </span>Mushly McMushmaster <<a href="mailto:norman-k-williams@uiowa.edu">norman-k-williams@uiowa.edu</a>><br>
<span style="font-weight:bold">Cc: </span>ITK <<a href="mailto:insight-developers@itk.org">insight-developers@itk.org</a>><br>
<span style="font-weight:bold">Subject: </span>Re: [ITK-dev] MatrixOffsetTransformBase::GetFixedParameters not ThreadSafe -- regression test?<br>
</div>
<div><br>
</div>
<div>
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">
Kent,
<div><br>
</div>
<div>Mutable member are evil.</div>
<div><br>
</div>
<div>Would it be possible to have the m_Center and the m_FixedArray point to the same block of data?</div>
<div><br>
</div>
<div>Brad</div>
<div><br>
<div>
<div>On Oct 24, 2014, at 12:59 PM, Williams, Norman K <<a href="mailto:norman-k-williams@uiowa.edu">norman-k-williams@uiowa.edu</a>> wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; font-size: 14px; font-family: Calibri, sans-serif;">
<div>The problem is worse than I thought.</div>
<div><br>
</div>
<div>CompositeTransform::GetFixedParameters() dynamically resizes the FixedParameters array, and then does a bunch of std::copy to copy fixed parameters out of its list of sub-transforms.</div>
<div><br>
</div>
<div>The only way to make that thread safe is to use a Mutex lock to guard where it changes the member variable.</div>
<div><br>
</div>
<span id="OLK_SRC_BODY_SECTION">
<div style="font-family: Calibri; font-size: 11pt; text-align: left; border-width: 1pt medium medium; border-style: solid none none; padding: 3pt 0in 0in; border-top-color: rgb(181, 196, 223);">
<span style="font-weight:bold">From: </span><Williams>, Mushly McMushmaster <<a href="mailto:norman-k-williams@uiowa.edu">norman-k-williams@uiowa.edu</a>><br>
<span style="font-weight:bold">Date: </span>Friday, October 24, 2014 at 11:45 AM<br>
<span style="font-weight:bold">To: </span>ITK <<a href="mailto:insight-developers@itk.org">insight-developers@itk.org</a>><br>
<span style="font-weight:bold">Subject: </span>[ITK-dev] MatrixOffsetTransformBase::GetFixedParameters not ThreadSafe -- regression test?<br>
</div>
<div><br>
</div>
<div>
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; font-size: 14px; font-family: Calibri, sans-serif;">
<div>I was going to address this bug that I just logged:</div>
<div><br>
</div>
<div><a href="https://issues.itk.org/jira/browse/ITK-3324">https://issues.itk.org/jira/browse/ITK-3324</a></div>
<div><br>
</div>
<div>Basically, if GetFixedParameters is called from multiple threads, it introduces a race to resize the internal fixed parameter array.</div>
<div><br>
</div>
<div>This was never a problem before Hans Johnson’s recent patch to propagate fixed parameters int Transform::GetInverse() methods. He added:</div>
<div><br>
</div>
<div>inverse->SetFixedParameters(this->GetFixedParameters());</div>
<div><br>
</div>
<div>which makes perfect sense.</div>
<div><br>
</div>
<div>I have a decent idea of how to make this better: Set the fixed parameters’ size in the constructor for MatrixOffsetTransformBase, and set the fixed parameters in SetCenter. I will review the other instances of GetFixedParameters and GetParameters to
see if they’re similarly thread-unsafe.</div>
<div><br>
</div>
<div>But how do you write a regression test for this?</div>
<div><br>
</div>
<div><br>
</div>
<br>
<br>
<hr>
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged. If you are not the intended recipient, you are hereby notified that any
retention, dissemination, distribution, or copying of this communication is strictly prohibited. Please reply to the sender that you have received the message in error, then delete it. Thank you.
<hr>
</div>
</div>
</span><br>
<br>
<hr>
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged. If you are not the intended recipient, you are hereby notified that any
retention, dissemination, distribution, or copying of this communication is strictly prohibited. Please reply to the sender that you have received the message in error, then delete it. Thank you.
<hr>
</div>
_______________________________________________<br>
Powered by <a href="http://www.kitware.com">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at<br>
<a href="http://www.kitware.com/opensource/opensource.html">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Kitware offers ITK Training Courses, for more information visit:<br>
<a href="http://kitware.com/products/protraining.php">http://kitware.com/products/protraining.php</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at:<br>
<a href="http://www.itk.org/Wiki/ITK_FAQ">http://www.itk.org/Wiki/ITK_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/mailman/listinfo/insight-developers">http://public.kitware.com/mailman/listinfo/insight-developers</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</span><br>
<br>
<hr>
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged. If you are not the intended recipient, you are hereby notified that any
retention, dissemination, distribution, or copying of this communication is strictly prohibited. Please reply to the sender that you have received the message in error, then delete it. Thank you.
<hr>
</body>
</html>