<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 11, 2013 at 8:12 AM, Björn Gustavsson <span dir="ltr"><<a href="mailto:bgustavsson@gmail.com" target="_blank">bgustavsson@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>On Tue, Apr 9, 2013 at 9:27 PM, Aliaksey Kandratsenka <span dir="ltr"><<a href="mailto:alkondratenko@gmail.com" target="_blank">alkondratenko@gmail.com</a>></span> wrote:<br>

</div><div class="gmail_extra"><div><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote"><div>I've updated patch to use erlang:now instead of os:timestamp.</div>


<div><br></div></div></div></div>
</blockquote></div><br></div>Thanks! We have now reviewed the code and have</div><div class="gmail_extra">found a minor potential issue.</div><div class="gmail_extra"><br></div><div class="gmail_extra">The thing is that if two timers are set to the same<br>


</div><div class="gmail_extra">value, they are now guaranteed to expire in the same</div><div class="gmail_extra">order that they were set. That is because erlang:now/0</div><div class="gmail_extra">returns unique values. (See timer_timeout/1.)</div>


<div class="gmail_extra"><br></div><div class="gmail_extra">With your patch, timers set to the same timeout</div><div class="gmail_extra">within the same millisecond will have identical</div><div class="gmail_extra">target times. Since the key in the ets table</div>


<div class="gmail_extra">(at least for one shot timers) is tuple with the</div><div class="gmail_extra">second element a reference, the ordering of the</div><div class="gmail_extra">refs will determine which key that erts:first/1 will</div>


<div class="gmail_extra">return if the target times are identical.</div><div class="gmail_extra"><br></div><div class="gmail_extra">In the current implementation, it *happens* to</div><div class="gmail_extra">
be true that if only and if Ref1 < Ref2, then</div><div class="gmail_extra">Ref1 was created before Ref2. But that property</div><div class="gmail_extra">is not guaranteed by the language definition and it</div>
<div class="gmail_extra">may be changed in a future release (e.g. to reduce</div><div class="gmail_extra">contention of the lock that is taken when refs are</div><div class="gmail_extra">created).</div><div class="gmail_extra">


<br></div><div class="gmail_extra">Therefore, I suggest fixing the bug by keeping</div><div class="gmail_extra">the original system_time/0 function and</div><div class="gmail_extra">round up when dividing by 1000. (The easiest</div>


<div class="gmail_extra">way to round up is to add 999 before dividing</div><div class="gmail_extra">by 1000.)</div></div></blockquote><div><br></div><div>That makes sense. Thanks for thorough review.</div><div><br></div>
<div style>I've updated patch.</div><div><br></div></div></div></div>