I'm not sure of the thinking of others regarding changes to the
verifiedIncompatible variables since, but my thinking in my initial
implementation was that they would only be set after a handshake, thus
we already had a connected to them and ARKs weren't needed. That won't
change until the peer re-handshakes after restarting with a different
version of the node's software.
Robert Hailey wrote:
>
> On Jan 2, 2008, at 7:38 AM, Florent Daigni?re wrote:
>
>>>>>> synchronized void updateShouldDisconnectNow() {
>>>>>> //FIXME: We should not update VERIFIED unless we HANDSHAKE WITH
>>>>>> THE NODE
>>>>>> - if (isConnected()) {
>>>>>> + if (isConnected() || verifiedIncompatibleOlderVersion
>>>>>> || verifiedIncompatibleNewerVersion) {
>>>>>> verifiedIncompatibleOlderVersion = forwardInvalidVersion();
>>>>>> verifiedIncompatibleNewerVersion = reverseInvalidVersion();
>>>>>> }
>>>>>
>>>>> I suggest you call isRoutable() instead
>>>>>
>>>>> NextGen$
>>>>
>>>> Hmm, on a latter thought, I don't understand why it helps... nor why we
>>>> are calling it from PacketSender
>>>>
>>>> What about getting rid of both the "if" branch and the call in
>>>> PacketSender ?
>>>
>>> Why not just use if(isConnected()) { ... } ? As far as I can see
>>> that is
>>> correct: if we're not connected, we don't care about verified*.
>>
>> You're suggesting to revert robert's patch, wich is fine by me... as I
>> said, I don't understand why it helps
>>
>> NextGen$
>
> 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).
>
> 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:
> (1) last-seen node version is incompatible (but before r16834, would
> set 'verified')
> (2) attempt handshakes, but don't fetch ark
> (3) IP has changed, so can't handshake
> (4) wont get new IP (ark), because older version
>
> 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).
>
> --
> Robert Hailey
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Devl mailing list
> Devl at freenetproject.org
> http://emu.freenetproject.org/cgi-bin/mailman/listinfo/devl
