<div dir="ltr"><br><br><div class="gmail_quote">On Wed, Aug 6, 2008 at 10:00 PM, Ceki Gulcu <span dir="ltr">&lt;listid@qos.ch&gt;</span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d"><br>
<br>
Joern Huxhorn wrote:<br>
&gt; Hi Ceki,<br>
&gt; some comments on the latest commit:<br>
<br>
</div><div class="Ih2E3d">&gt; statusList should be final if it&#39;s later used for synchronization. At<br>
</div><div class="Ih2E3d">&gt; As statusList, statusListenerList should be final.<br>
<br>
</div>OK.<br>
<div class="Ih2E3d">&gt;&gt;<br>
&gt;&gt; - &nbsp;public Iterator&lt;Status&gt; iterator() {<br>
&gt;&gt; - &nbsp; &nbsp;return statusList.iterator();<br>
&gt;&gt; + &nbsp;public synchronized List&lt;Status&gt; getCopyOfStatusList() {<br>
&gt;&gt; + &nbsp; &nbsp;synchronized (statusList) {<br>
&gt;&gt; + &nbsp; &nbsp; &nbsp;return new ArrayList&lt;Status&gt;(statusList);<br>
&gt;&gt; + &nbsp; &nbsp;}<br>
&gt;&gt; &nbsp; &nbsp;}<br>
&gt;&gt;<br>
&gt; I guess the method shouldn&#39;t be synchronized anymore since<br>
&gt; synchronization is done on statusList.<br>
<br>
</div>&nbsp;From what I can read from the ArrayList constructor taking a collection,<br>
copying of the collection passed as parameter is not synchronized. Moreover, the<br>
mutex used by the SyncronizedArrayList is the object itself (the<br>
SyncronizedArrayList instance), so synchronized(statusList){...} is necessary.<br>
<div class="Ih2E3d"></div></blockquote><div><br>Yes, definitely @ synchronized(statusList).<br>I just think that <br>public synchronized List&lt;Status&gt; getCopyOfStatusList()<br>should be<br>public List&lt;Status&gt; getCopyOfStatusList() <br>
so there is no risk of deadlock.<br><br>I haven&#39;t checked if there *is* a risk of deadlock but there *could* be under certain circumstances, i.e. if there would be another place that synchronizes on statusList first and on Object later. I don&#39;t think there is such a piece of code but it&#39;s generally a good idea to not synchronize/holding two locks if not absolutely necessary. It&#39;s also a bit faster.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="Ih2E3d"><br>
&gt;&gt; + &nbsp;public void remove(StatusListener listener) {<br>
&gt;&gt; + &nbsp; &nbsp;statusListenerList.add(listener);<br>
&gt;&gt; + &nbsp;}<br>
&gt;&gt;<br>
&gt; Should be statusListenerList.remove(listener);<br>
<br>
</div>Four eyes are better than two. thank you!<br>
</blockquote><div><br>Your welcome,<br>Joern.</div></div><br></div>