On Jan 12, 2008, at 2:01 PM, Matthew Toseland wrote:
> On Friday 11 January 2008 22:46, Robert Hailey wrote:
>>
>> On Jan 11, 2008, at 4:06 PM, Matthew Toseland wrote:
>>
>>> On Wednesday 09 January 2008 22:10, robert at freenetproject.org wrote:
>>>> Author: robert
>>>> Date: 2008-01-09 22:10:36 +0000 (Wed, 09 Jan 2008)
>>>> New Revision: 16987
>>>>
>>>> Modified:
>>>> trunk/freenet/src/freenet/io/xfer/BlockTransmitter.java
>>>> Log:
>>>> don't assume everything has been transmitted when the transmitter
>>>> oversleeps
>>>> - should get rid of "haven't heard from receiver in 1m55s." message
>>>> - the transmitter still oversleeps (2 minutes?!), and is not woken
>>>> up by
>>> notifyAll() because it is in the throttle function: delay()
>>>
>>> Why does the below help?
>>
>> I think it was actually because delay() was deadlocking, not
>> oversleeping. In any case, this helps then because we will not then
>> see an old timeAllSent and assume that the receiver is dead ("haven't
>> heard from the receiver").
>
> I still don't understand this - timeAllSent isn't set until not only
> have we
> sent all the packets but we've also received all the packets.
> Therefore,
> packetReceived() won't be called after that, because
> packetReceived() means
> we've received a new packet from the previous sender. I can see that
> we'd
> want to reset it in some other cases, but not here.
You are 99.9% right. Logically it should be totally redundant (if ever
we are receiving a packet, we have not sent everything), but this
actually can have an effect if we receive duplicate packets (which due
to the requeueing bug & retransmit requests was possible). Arguably it
would be better to not call the callback on receiving a duplicate
packet, but both of these fixes are overkill for a problem about to go
away (the only reason I could see that the BlockReceiver would be
asking for a missing packet is if it was endlessly requeued).
On a side note, it may be the case that the block transmitter threads
are not properly exiting in the trunk, I need to take a closer look at
this before the next build.
--
Robert Hailey
>>>> Modified: trunk/freenet/src/freenet/io/xfer/BlockTransmitter.java
>>>> ===================================================================
>>>> --- trunk/freenet/src/freenet/io/xfer/BlockTransmitter.java
>>>> 2008-01-09
>>> 18:33:46 UTC (rev 16986)
>>>> +++ trunk/freenet/src/freenet/io/xfer/BlockTransmitter.java
>>>> 2008-01-09
>>> 22:10:36 UTC (rev 16987)
>>>> @@ -192,6 +192,7 @@
>>>> public void packetReceived(int
>>>> packetNo) {
>>>> synchronized(_senderThread) {
>>>> _unsent.addLast(new
>>>> Integer(packetNo));
>>>> + timeAllSent = -1;
>>>>
>>>> _sentPackets.setBit(packetNo, false);
>>>>
>>>> _senderThread.notifyAll();
>>>> }
>>>
