View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0002406GNUnetobsoletepublic2012-06-07 11:092024-05-03 14:02
ReporterLRN Assigned ToLRN  
PriorityurgentSeveritymajorReproducibilityrandom
Status closedResolutionfixed 
PlatformW32OSNTOS Version6.1.7601
Product VersionGit master 
Target Version0.9.4Fixed in Version0.9.4 
Summary0002406: Core fails to process all peerinfo messages when more than one are queued
DescriptionCORE: Core asks peerinfo for a HELLO for PEER1, schedules a request to be sent to peerinfo.
CORE: It then also asks peerinfo for a HELLO for PEER2. Seeing that a request is already scheduled, it puts another request into the queue.
CORE: Scheduled request is sent.
PEIN: Peerinfo receives HELLO request for PEER1.
CORE: Next request (for PEER2's HELLO) in the queue is sent to peerinfo too.
PEIN: Peerinfo starts sending back HELLO for PEER1.
PEIN: Sends back 128 bytes
PEIN: Sends back 128 bytes
PEIN: Sends back 128 bytes
PEIN: Sends back 4 bytes (end of an address list for PEER1)
PEIN: Peerinfo starts reading again, receives HELLO request for PEER2.
PEIN: Sends back 128 bytes
PEIN: Sends back 128 bytes
CORE: Finally wakes up and realizes that peerinfo sent something back. Reads 128+128+128+4+128+128=644 bytes
PEIN: Sends back 128 bytes
PEIN: Sends back 4 bytes (end of an address list for PEER2)
CORE: Processes 644 byte-long buffer. Sees a 384-byte-long HELLO for PEER1.
CORE: Processes PEER1's HELLO.
CORE: Processes 644-384=260 byte-long byffer. Sees a 4-byte End Of List Of Peers.
CORE: Stops reading from peerinfo. Still has 256 bytes in the buffer, and still haven't read 132 bytes from peerinfo. Does not get a HELLO for PEER2. Fails to connect to PEER2. Fails the test.

In cases when a core service manages to get both HELLOs, it doesn't ask peerinfo for two HELLOs at once - it asks for one, receives one, then asks for another one, and receives it too.
Steps To ReproduceRun test_dht_line a few times on W32. It will show this kind of behaviour eventually.
Additional InformationI have log files, if you're interested.
TagsNo tags attached.
Attached Files
0001-Make-sure-peerinfo-API-keeps-receiving-as-long-as-th.patch (1,027 bytes)    
From 25c895f688089de4fc570dd9f9a82d5d738581b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1=D1?=
 =?UTF-8?q?=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= <lrn1986@gmail.com>
Date: Thu, 7 Jun 2012 14:45:42 +0400
Subject: [PATCH] Make sure peerinfo API keeps receiving as long as there are
 contexts

---
 src/peerinfo/peerinfo_api.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/src/peerinfo/peerinfo_api.c b/src/peerinfo/peerinfo_api.c
index c13224e..d6ba0e6 100644
--- a/src/peerinfo/peerinfo_api.c
+++ b/src/peerinfo/peerinfo_api.c
@@ -515,6 +515,12 @@ peerinfo_handler (void *cls, const struct GNUNET_MessageHeader *msg)
     trigger_transmit (h);
     if (NULL != cb)
       cb (cb_cls, NULL, NULL, NULL);
+    if (h->ic_head != NULL)
+    {
+      h->in_receive = GNUNET_YES;
+      GNUNET_CLIENT_receive (h->client, &peerinfo_handler, h,
+          GNUNET_TIME_absolute_get_remaining (h->ic_head->timeout));
+    }
     return;
   }
 
-- 
1.7.4
0001-Make-sure-peerinfo-API-keeps-receiving-as-long-as-th.patch (1,027 bytes)   

Activities

LRN

2012-06-07 11:50

reporter   ~0006006

At the moment i think it's a bug in peerinfo API.

LRN

2012-06-07 12:46

reporter   ~0006007

Uploaded a fix - 0001-Make-sure-peerinfo-API-keeps-receiving-as-long-as-th.patch

Christian Grothoff

2012-06-07 21:29

manager   ~0006014

I don't see how this helps. A line above we call 'trigger_transmit', which will end up calling 'do_transmit', which calls 'ac->cont' which if we get a response will call 'iterator_start_receive' which will trigger 'GNUNET_CLIENT_receive' already. In which case is your change actually helping?

LRN

2012-06-07 22:37

reporter   ~0006022

LRN: trigger_transmit() won't schedule do_transmit(), because it hits "No requests queued" clause
LRN: do_transmit is for requests pending to be sent
LRN: however, in my case the request is already sent
LRN: (peerinfo api sends two requests in a row)
LRN: once that happens, ac_head is NULL, but ic_head isn't

That is, peerinfo API only works correctly when it goes like this:
1) Send a request
2) Receive the reply
3) Send a request
4) Receive the reply
...
When it goes like this:
1) Send a request
2) Send a request
3) Receive the reply
it does not receive the second reply, because it does not consider non-NULL ic_head to be a reason to keep receiving data.

Christian Grothoff

2012-06-07 23:10

manager   ~0006023

Ah, I see. The goal was that peerinfo shouldn't send two requests in a row. But clearly that is possible: while trigger_transmit checks if a request is pending ('in_receive'), that flag may be set AFTER the check (i.e. by 'ac->cont' in line 375). Calling that continuation before 'trigger_transmit' might solve that, but I have to see if that might cause other trouble first...

Christian Grothoff

2012-06-07 23:10

manager   ~0006024

As 'cont' might be user-defined (and could include a peerinfo-disconnect call), calling 'trigger_transmit' after it is not generally safe(r). :-(.

Christian Grothoff

2012-06-08 15:04

manager   ~0006026

Fixed with minor variation in SVN 21802.

Issue History

Date Modified Username Field Change
2012-06-07 11:09 LRN New Issue
2012-06-07 11:50 LRN Note Added: 0006006
2012-06-07 12:46 LRN File Added: 0001-Make-sure-peerinfo-API-keeps-receiving-as-long-as-th.patch
2012-06-07 12:46 LRN Note Added: 0006007
2012-06-07 21:29 Christian Grothoff Note Added: 0006014
2012-06-07 21:29 Christian Grothoff Assigned To => LRN
2012-06-07 21:29 Christian Grothoff Status new => feedback
2012-06-07 22:37 LRN Note Added: 0006022
2012-06-07 22:37 LRN Status feedback => assigned
2012-06-07 22:39 LRN Category core service => peerinfo service
2012-06-07 23:10 Christian Grothoff Note Added: 0006023
2012-06-07 23:10 Christian Grothoff Note Added: 0006024
2012-06-07 23:14 Christian Grothoff Assigned To LRN => Christian Grothoff
2012-06-07 23:14 Christian Grothoff Priority normal => urgent
2012-06-07 23:15 Christian Grothoff Target Version => 0.9.4
2012-06-08 15:04 Christian Grothoff Note Added: 0006026
2012-06-08 15:04 Christian Grothoff Status assigned => resolved
2012-06-08 15:04 Christian Grothoff Fixed in Version => 0.9.4
2012-06-08 15:04 Christian Grothoff Resolution open => fixed
2012-06-08 15:04 Christian Grothoff Assigned To Christian Grothoff => LRN
2012-11-05 18:34 Christian Grothoff Status resolved => closed
2024-05-03 14:02 Christian Grothoff Category peerinfo service => obsolete