<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jan 2, 2008, at 7:38 AM, Florent Daignière wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Courier; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0; "><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">synchronized void updateShouldDisconnectNow() {<br></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><span class="Apple-tab-span" style="white-space: pre; "> </span>//FIXME: We should not update VERIFIED unless we HANDSHAKE WITH THE<span class="Apple-converted-space"> <span class="Apple-style-span" style="-webkit-text-stroke-width: -1; ">NODE</span></span></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">-<span class="Apple-tab-span" style="white-space: pre; "> </span>if (isConnected()) {<br></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">+<span class="Apple-tab-span" style="white-space: pre; "> </span>if (isConnected() || verifiedIncompatibleOlderVersion ||<span class="Apple-converted-space"> <span class="Apple-style-span" style="-webkit-text-stroke-width: -1; ">verifiedIncompatibleNewerVersion) {</span></span></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><span class="Apple-tab-span" style="white-space: pre; "> </span><span class="Apple-tab-span" style="white-space: pre; "> </span>verifiedIncompatibleOlderVersion = forwardInvalidVersion();<br></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><span class="Apple-tab-span" style="white-space: pre; "> </span><span class="Apple-tab-span" style="white-space: pre; "> </span>verifiedIncompatibleNewerVersion = reverseInvalidVersion();<br></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><span class="Apple-tab-span" style="white-space: pre; "> </span>}<br></blockquote></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">I suggest you call isRoutable() instead<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">NextGen$<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Hmm, on a latter thought, I don't understand why it helps... nor why we<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">are calling it from PacketSender<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">What about getting rid of both the "if" branch and the call in<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">PacketSender ?<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Why not just use if(isConnected()) { ... } ? As far as I can see that is<span class="Apple-converted-space"> </span><br></blockquote><blockquote type="cite">correct: if we're not connected, we don't care about verified*.<br></blockquote><br>You're suggesting to revert robert's patch, wich is fine by me... as I<br>said, I don't understand why it helps<br><br>NextGen$</span></blockquote></div><br><div>This function is called at various times (connected or not). If the only condition baring updating the 'verified' flags is: "isConnected()"... then the added condition could SET the verified-incompatible flags but could not UNSET them unless connected. The purpose for adding the extra OR's to the conditional is to allow this function to clear the verified-incompatible flags even if not connected (e.g. new ark fetched, node has a new version, clear the verified-flag).</div><div><br class="webkit-block-placeholder"></div><div>I have had what appears to be this 'deadlock' twice, the first with a laptop (whose IP and node-version changed at the same time), and it is this:</div><div>(1) last-seen node version is incompatible (but before r16834, would set 'verified')</div><div>(2) attempt handshakes, but don't fetch ark</div><div>(3) IP has changed, so can't handshake</div><div>(4) wont get new IP (ark), because older version</div><div><br class="webkit-block-placeholder"></div><div><div>A better solution to this might simply be to fetch arks even on version incompatibility, but (for caution) behavior #2 seemed intentional (dont fetch ark on incompatible version), whereas #1 did not, and brought it closer to the comments (i.e. don't set 'verified' unless handshake).</div><div><br class="webkit-block-placeholder"></div><div>--</div><div>Robert Hailey</div><div><br class="webkit-block-placeholder"></div></div></body></html>