[freenet-dev] [freenet-cvs] r16834 - trunk/freenet/src/freenet/node

[David Sowder]

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