View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001815 | GNUnet | core service | public | 2011-10-02 21:55 | 2011-10-31 12:00 |
| Reporter | Christian Grothoff | Assigned To | Christian Grothoff | ||
| Priority | urgent | Severity | major | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Summary | 0001815: core sends NotifyTraffic message (NOTIFY_INBOUND) for peer that is not connected | ||||
| Description | DHT multipeer test failed with the following assertion: Sep 30 17:31:55-325006 dht-25024 ERROR Assertion failed at core_api.c:1050. Sep 30 17:32:14-914708 dht-25132 ERROR Assertion failed at core_api.c:1050. This is on the arm buildbot. | ||||
| Tags | No tags attached. | ||||
| Attached Files | core.diff (6,196 bytes)
Index: gnunet-service-core.c
===================================================================
--- gnunet-service-core.c (revision 17183)
+++ gnunet-service-core.c (working copy)
@@ -1062,6 +1062,8 @@
/* check if neighbour queue is empty enough! */
if (n != &self)
{
+ if (n->status == PEER_STATE_KEY_CONFIRMED)
+ return; /* not yet ready */
queue_size = 0;
mqe = n->messages;
while (mqe != NULL)
@@ -1221,8 +1223,9 @@
n = &self;
else
n = find_neighbour (&req->peer);
- if ((n == NULL) || (GNUNET_YES != n->is_connected) ||
- (n->status != PEER_STATE_KEY_CONFIRMED))
+ if ( (n == NULL) || (GNUNET_YES != n->is_connected) ||
+ ( (n->status != PEER_STATE_KEY_CONFIRMED) &&
+ (n->status != PEER_STATE_KEY_RECEIVED) ) )
{
/* neighbour must have disconnected since request was issued,
* ignore (client will realize it once it processes the
@@ -1313,13 +1316,13 @@
sizeof (struct GNUNET_TRANSPORT_ATS_Information) * n->ats_count);
ats[n->ats_count].type = htonl (GNUNET_TRANSPORT_ATS_ARRAY_TERMINATOR);
ats[n->ats_count].value = htonl (0);
+ cnm->peer = n->peer;
if (n->status == PEER_STATE_KEY_CONFIRMED)
{
#if DEBUG_CORE_CLIENT
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG, "Sending `%s' message to client.\n",
"NOTIFY_CONNECT");
#endif
- cnm->peer = n->peer;
send_to_client (c, &cnm->header, GNUNET_NO);
}
return GNUNET_OK;
@@ -1514,7 +1517,8 @@
struct ConnectNotifyMessage *cnm;
cnm = (struct ConnectNotifyMessage *) buf;
- if (n->status != PEER_STATE_KEY_CONFIRMED)
+ if ( (n->status != PEER_STATE_KEY_CONFIRMED) &&
+ (n->status != PEER_STATE_KEY_RECEIVED) )
return GNUNET_OK;
size =
sizeof (struct ConnectNotifyMessage) +
@@ -3512,11 +3516,7 @@
uint32_t ats_count)
{
struct PongMessage t;
- struct ConnectNotifyMessage *cnm;
struct GNUNET_CRYPTO_AesInitializationVector iv;
- char buf[GNUNET_SERVER_MAX_MESSAGE_SIZE - 1];
- struct GNUNET_TRANSPORT_ATS_Information *mats;
- size_t size;
#if DEBUG_HANDSHAKE
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
@@ -3576,6 +3576,7 @@
("# Session keys confirmed via PONG"), 1,
GNUNET_NO);
n->status = PEER_STATE_KEY_CONFIRMED;
+ schedule_peer_messages (n);
{
struct GNUNET_MessageHeader *hdr;
@@ -3604,30 +3605,6 @@
n->retry_set_key_task = GNUNET_SCHEDULER_NO_TASK;
}
update_neighbour_performance (n, ats, ats_count);
- size =
- sizeof (struct ConnectNotifyMessage) +
- (n->ats_count) * sizeof (struct GNUNET_TRANSPORT_ATS_Information);
- if (size >= GNUNET_SERVER_MAX_MESSAGE_SIZE)
- {
- GNUNET_break (0);
- /* recovery strategy: throw away performance data */
- GNUNET_array_grow (n->ats, n->ats_count, 0);
- size =
- sizeof (struct PeerStatusNotifyMessage) +
- n->ats_count * sizeof (struct GNUNET_TRANSPORT_ATS_Information);
- }
- cnm = (struct ConnectNotifyMessage *) buf;
- cnm->header.size = htons (size);
- cnm->header.type = htons (GNUNET_MESSAGE_TYPE_CORE_NOTIFY_CONNECT);
- cnm->ats_count = htonl (n->ats_count);
- cnm->peer = n->peer;
- mats = &cnm->ats;
- memcpy (mats, n->ats,
- n->ats_count * sizeof (struct GNUNET_TRANSPORT_ATS_Information));
- mats[n->ats_count].type = htonl (GNUNET_TRANSPORT_ATS_ARRAY_TERMINATOR);
- mats[n->ats_count].value = htonl (0);
- send_to_all_clients (&cnm->header, GNUNET_NO,
- GNUNET_CORE_OPTION_SEND_CONNECT);
process_encrypted_neighbour_queue (n);
/* fall-through! */
case PEER_STATE_KEY_CONFIRMED:
@@ -3667,6 +3644,10 @@
struct PingMessage *ping;
struct PongMessage *pong;
enum PeerStateMachine sender_status;
+ struct ConnectNotifyMessage *cnm;
+ char buf[GNUNET_SERVER_MAX_MESSAGE_SIZE - 1];
+ struct GNUNET_TRANSPORT_ATS_Information *mats;
+ size_t size;
#if DEBUG_CORE
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
@@ -3761,6 +3742,32 @@
}
update_neighbour_performance (n, ats, ats_count);
sender_status = (enum PeerStateMachine) ntohl (m->sender_status);
+ if ( (n->status == PEER_STATE_DOWN) ||
+ (n->status == PEER_STATE_KEY_SENT) )
+ {
+ size =
+ sizeof (struct ConnectNotifyMessage) +
+ (n->ats_count) * sizeof (struct GNUNET_TRANSPORT_ATS_Information);
+ if (size >= GNUNET_SERVER_MAX_MESSAGE_SIZE)
+ {
+ GNUNET_break (0);
+ /* recovery strategy: throw away performance data */
+ GNUNET_array_grow (n->ats, n->ats_count, 0);
+ size =
+ sizeof (struct PeerStatusNotifyMessage) +
+ n->ats_count * sizeof (struct GNUNET_TRANSPORT_ATS_Information);
+ }
+ cnm = (struct ConnectNotifyMessage *) buf;
+ cnm->header.size = htons (size);
+ cnm->header.type = htons (GNUNET_MESSAGE_TYPE_CORE_NOTIFY_CONNECT);
+ cnm->ats_count = htonl (n->ats_count);
+ cnm->peer = n->peer;
+ mats = &cnm->ats;
+ memcpy (mats, n->ats,
+ n->ats_count * sizeof (struct GNUNET_TRANSPORT_ATS_Information));
+ mats[n->ats_count].type = htonl (GNUNET_TRANSPORT_ATS_ARRAY_TERMINATOR);
+ mats[n->ats_count].value = htonl (0);
+ }
switch (n->status)
{
case PEER_STATE_DOWN:
@@ -3769,9 +3776,14 @@
GNUNET_log (GNUNET_ERROR_TYPE_DEBUG,
"Responding to `%s' with my own key.\n", "SET_KEY");
#endif
+ send_to_all_clients (&cnm->header, GNUNET_NO,
+ GNUNET_CORE_OPTION_SEND_CONNECT);
send_key (n);
break;
case PEER_STATE_KEY_SENT:
+ send_to_all_clients (&cnm->header, GNUNET_NO,
+ GNUNET_CORE_OPTION_SEND_CONNECT);
+ /* fall-through */
case PEER_STATE_KEY_RECEIVED:
n->status = PEER_STATE_KEY_RECEIVED;
if ((sender_status != PEER_STATE_KEY_RECEIVED) &&
@@ -4454,7 +4466,8 @@
return;
}
GNUNET_break (n->is_connected == GNUNET_YES);
- if (n->status == PEER_STATE_KEY_CONFIRMED)
+ if ( (n->status == PEER_STATE_KEY_CONFIRMED) ||
+ (n->status == PEER_STATE_KEY_RECEIVED) )
{
GNUNET_STATISTICS_update (stats,
gettext_noop ("# peers disconnected due to transport disconnect"), 1,
| ||||
| related to | 0001745 | closed | Christian Grothoff | core does not process message type map messages yet |
|
|
This failure, despite being handled with a 'reconnect', also seems to correlate with DHT mutlipeer test failures. |
|
|
send_p2p_message_to_client (in gnunet-service-core) asserts that the neighbour "is_connected", but not that the status is "PEER_STATE_KEY_CONFIRMED". However, "is_connected" is about the transport-level connection, what matters to clients is "PEER_STATE_KEY_CONFIRMED". So at least the assertion is insufficient. (A stronger assertion should probably be placed in "deliver_message"). |
|
|
With the new assertions, I now get: Oct 04 20:12:29-260675 core-42803 ERROR Assertion failed at gnunet-service-core.c:3892. |
|
|
Other peer is likely in 'confirmed', we are still in 'key_received'. Solution: 1) notify clients of connection upon 'key_received' 2) delay messages in other direction until 'key_confirmed' 3) intensify PING-ing if this happens. |
|
|
Attached a diff that "should" fix the issue (notify clients earlier, delay sending further), but instead just breaks all the tests... |
|
|
My current feeling is that this should be solved by addressing 0001745: peers are only seen as 'connected' after the type map received matching entries -- and only then are they allowed to transmit messages of those types. |
|
|
This is now fixed in gnunet-service-core-new (which, once more testing is done, will replace the old gnunet-service-core). |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2011-10-02 21:55 | Christian Grothoff | New Issue | |
| 2011-10-02 21:55 | Christian Grothoff | Status | new => assigned |
| 2011-10-02 21:55 | Christian Grothoff | Assigned To | => Christian Grothoff |
| 2011-10-02 21:57 | Christian Grothoff | Note Added: 0004666 | |
| 2011-10-04 13:45 | Christian Grothoff | Target Version | => 0.9.0pre4 |
| 2011-10-04 15:28 | Christian Grothoff | Priority | normal => urgent |
| 2011-10-04 15:46 | Christian Grothoff | Note Added: 0004687 | |
| 2011-10-04 21:42 | Christian Grothoff | Note Added: 0004688 | |
| 2011-10-04 22:20 | Christian Grothoff | Note Added: 0004689 | |
| 2011-10-04 23:08 | Christian Grothoff | File Added: core.diff | |
| 2011-10-04 23:09 | Christian Grothoff | Note Added: 0004690 | |
| 2011-10-07 13:29 | Christian Grothoff | Relationship added | related to 0001745 |
| 2011-10-07 13:30 | Christian Grothoff | Note Added: 0004694 | |
| 2011-10-11 14:58 | Christian Grothoff | Note Added: 0004700 | |
| 2011-10-11 14:58 | Christian Grothoff | Status | assigned => resolved |
| 2011-10-11 14:58 | Christian Grothoff | Fixed in Version | => 0.9.0pre4 |
| 2011-10-11 14:58 | Christian Grothoff | Resolution | open => fixed |
| 2011-10-31 12:00 | Christian Grothoff | Status | resolved => closed |
