Enet Send and recieve in two threads will crash

[zhengchengbin610]

I encapsulated the sending and receiving of Enet, the receiving function will be called in a thread all the time, and the sending will be called in other threads, so there will be a segmentation error. my part of code is as follows. I now want know mybe I cannot understand the Enet library ? for this case ,if one why to fix to use mutex lock ?

typedef struct _ASD_RELIABLE_UDP
{
ENetHost *host;
ENetPeer *peer;
}ASD_RELIABLE_UDP;

init function:
int asd_reliable_udp_init(ASD_RELIABLE_UDP *udp, enet_uint32 sockfd, enet_uint8 *ip, enet_uint16 port)
{
ENetEvent event;
int retry = 0;
if(enet_initialize())
{
return -1;
}
//memset(&(udp->event), 0, sizeof(udp->event));
pthread_mutex_init(&(udp->hmutex),NULL);

if (NULL == ip && 0 != port)
{
	ENetAddress address;
	address.host=ENET_HOST_ANY;
	address.port=port;

	udp->host = enet_host_create(&address, sockfd, 1, 0,0,0);/*init server*/
	
	//udp->host->checksum = asd_check_buffer_sums;
	printf("==== rudp host:%x\n ",udp->host);
	if(udp->host) return 0;
	else return -1;
}
else if (NULL != ip && 0 != port)
{
	int ret;
	
	udp->host = enet_host_create(NULL, sockfd, 1, 0,0,0);/*init client*/

	ENetAddress svraddr;
	memset(&svraddr, 0, sizeof(ENetAddress));

	if (0 != enet_address_set_host(&svraddr,ip))
	{
		ret = -1;
		goto final;
		//return -1;
	}
	svraddr.port = port;

again:
udp->peer = enet_host_connect(udp->host, &svraddr, 2,0);
if (NULL == udp->peer)
{
ret = -1;
goto final;
//return -1;
}
if (enet_host_service (udp->host, &event, 1000) > 0 && event.type == ENET_EVENT_TYPE_CONNECT)
{
fprintf(stdout,"#### connect host success ....\n");

		ret = 0;
		goto final;
		//return 0;
	}
	else
	{
		enet_peer_reset (udp->peer);
		if(retry < 3){
			retry ++ ;
			fprintf(stderr," ### connect host again ....\n");
			goto again;
		}
		else{
			ret = -1;
			goto final;
			//return -1;
		}
	}

final:
return ret;
}
}

send function:

int asd_reliable_udp_send(ASD_RELIABLE_UDP *udp, const void *data, size_t len)
{
ENetEvent event;
if (NULL == udp->host || NULL == udp->peer || NULL == data || len <= 0)
{
fprintf(stderr,"param is error,host:%x,event.peer:%x,data:%x,len:%d\n",udp->host,udp->peer,len);
return -1;
}

ENetPacket *packet = enet_packet_create(data,len,ENET_PACKET_FLAG_RELIABLE);
if (NULL == packet)
{
	fprintf(stderr,"enet_packet_create failed\n");
	return -1;
}

if (0 != enet_peer_send(udp->peer,1,packet))
{
	fprintf(stderr,"enet_peer_send failed,errno:%d\n",errno);
	enet_packet_destroy(packet);
	return -1;
}

{
	enet_host_flush (udp->host);
}
return len;

}

int asd_reliable_udp_recv(ASD_RELIABLE_UDP *udp, void *data, size_t len,size_t timeout)
{
int datalen = 0;
ENetEvent event;
if (NULL == udp || NULL == udp->host || NULL == data || len <= 0)
{
return -1;
}
int err = enet_host_service(udp->host, &event, timeout);
if(err >= 0)
{
if (event.type == ENET_EVENT_TYPE_NONE)
{
//fprintf(stderr,"ENET_EVNET_TYPE_NONE ..:%d\n",errno);
goto fin;
}
else if (event.type == ENET_EVENT_TYPE_CONNECT)
{
goto fin;
}
else if(event.type == ENET_EVENT_TYPE_RECEIVE)
{
//fprintf(stderr,"Request len:%d,dataLeng:%d\n",len,event.packet->dataLength);
memcpy(data, event.packet->data, event.packet->dataLength);
datalen = event.packet->dataLength;
enet_packet_destroy(event.packet);
goto fin;
}
else if (event.type == ENET_EVENT_TYPE_DISCONNECT)
{
//fprintf(stderr,"event disconnect ****\n");
return -ENET_EVENT_TYPE_DISCONNECT;
}
}
fin:
return datalen;
}

[IceflowRE]

Could you put the code in code blocks, then its easier to read.

[zhengchengbin610]

@IceflowRE I use the code block comment again

typedef struct _ASD_RELIABLE_UDP
{
ENetHost *host;
ENetPeer *peer;
}ASD_RELIABLE_UDP;



int apeman_reliable_udp_init(APEMAN_RELIABLE_UDP *udp, enet_uint32 sockfd, enet_uint8 *ip, enet_uint16 port)
{
	ENetEvent event;
	int retry = 0;
	if(enet_initialize())
	{
	   return -1;
	}
	if (NULL == ip && 0 != port)
	{
		ENetAddress address;
		address.host=ENET_HOST_ANY;
		address.port=port;
		udp->host = enet_host_create(&address, sockfd, 1, 0,0,0);/*init server*/
		printf("==== rudp host:%x,ENetHost:%d, ENetPeer:%d\n ",udp->host,sizeof(ENetHost),sizeof(ENetPeer));
		if(udp->host) return 0;
		else return -1;
	}
	else if (NULL != ip && 0 != port)
	{
		int ret;
		udp->host = enet_host_create(NULL, sockfd, 2, 0,0,0);/*init client*/
		
		ENetAddress svraddr;
		memset(&svraddr, 0, sizeof(ENetAddress));

		if (0 != enet_address_set_host(&svraddr,ip))
		{
			ret = -1;
			goto final;
		}
		svraddr.port = port;
again:
		udp->peer = enet_host_connect(udp->host, &svraddr, 2,0);
		if (NULL == udp->peer)
		{
			ret = -1;
			goto final;
		}
		if (enet_host_service(udp->host, &event, 3000) > 0 && event.type == ENET_EVENT_TYPE_CONNECT)
		{
			fprintf(stdout,"#### connect host success ....\n");
			ret = 0;
			goto final;
		}
		else
		{
			enet_peer_reset (udp->peer);

			if(retry < 3){
				retry ++ ;
				fprintf(stderr," ### connect host again ....\n");
				goto again;
			}
			else{
				ret = -1;
				goto final;
			}
		}

	final:
		return ret;
	}
}

int apeman_reliable_udp_accept(APEMAN_RELIABLE_UDP *udp)
{
	int count=0;
	if (NULL == udp || NULL == udp->host)
	{
		return -1;
	}
	ENetEvent event;	
	int err;
again:
	err = enet_host_service(udp->host, &event, 1000);
	
	if (err >= 0) 
	{
		if (event.type == ENET_EVENT_TYPE_NONE)
		{
			printf("ENET_EVENT_TYPE_NONE\n");
			if(count <= 3){
				count++;
				goto again; 
			}
			else //break;
			return -1;
		}
		else if (event.type == ENET_EVENT_TYPE_CONNECT)
		{
			printf("ENET_EVENT_TYPE_CONNECT\n");
                        udp->peer = event.peer;
			return 0;
		}
	    else if (event.type == ENET_EVENT_TYPE_RECEIVE)
		{
			printf("ENET_EVENT_TYPE_RECEIVE...\n");
                       return -1;
		}
		else if (event.type == ENET_EVENT_TYPE_DISCONNECT)
		{
			return -1;
		}
	}
	return -1;
}


int asd_reliable_udp_send(ASD_RELIABLE_UDP *udp, const void *data, size_t len)
{
  ENetEvent event;
  if (NULL == udp->host || NULL == udp->peer || NULL == data || len <= 0)
  {
     fprintf(stderr,"param is error,host:%x,event.peer:%x,data:%x,len:%d\n",udp->host,udp->peer,len);
     return -1;
  }

  ENetPacket *packet = enet_packet_create(data,len,ENET_PACKET_FLAG_RELIABLE);
  if (NULL == packet)
  {
	  fprintf(stderr,"enet_packet_create failed\n");
	  return -1;
  }

  if (0 != enet_peer_send(udp->peer,1,packet))
  {
	  fprintf(stderr,"enet_peer_send failed,errno:%d\n",errno);
	  enet_packet_destroy(packet);
	  return -1;
  }
  {
	enet_host_flush (udp->host);
  }
  return len;
}

int asd_reliable_udp_recv(ASD_RELIABLE_UDP *udp, void *data, size_t len,size_t timeout)
{
  int datalen = 0;
  ENetEvent event;
  if (NULL == udp || NULL == udp->host || NULL == data || len <= 0)
  {
    return -1;
  }
  int err = enet_host_service(udp->host, &event, timeout);
  if(err >= 0)
  {
      if (event.type == ENET_EVENT_TYPE_NONE)
      {
        goto fin;
      }
    else if (event.type == ENET_EVENT_TYPE_CONNECT)
    {
      goto fin;
    }
    else if(event.type == ENET_EVENT_TYPE_RECEIVE)
    {
      //fprintf(stderr,"Request len:%d,dataLeng:%d\n",len,event.packet->dataLength);
      memcpy(data, event.packet->data, event.packet->dataLength);
      datalen = event.packet->dataLength;
      enet_packet_destroy(event.packet);
      goto fin;
    }
    else if (event.type == ENET_EVENT_TYPE_DISCONNECT)
    {
      //fprintf(stderr,"event disconnect ****\n");
      return -ENET_EVENT_TYPE_DISCONNECT;
    }
  }
  fin:
  return datalen;
}

[bjorn]

Note that you can edit your comments.

[zhengchengbin610]

Sorry, I know ,I misoperation just

[nxrighthere]

ENet is not thread-safe and never was. The only functionality that can be used relatively safely without accessing the same data from different threads is enet_packet_create() and enet_packet_destroy(). For anything else, you need synchronization mechanisms or inter-thread message-based communication without contention.

[zhengchengbin610]

Ok, I will use the mutex lock synchronization. I have another question is Enet library if a high speed to transmit data ? I have seen many enet data compared with other reliable udp, like kcp, can I modify what algorithm to improve the sending speed?

[nxrighthere]

It depends on many factors, including how well the library is integrated and utilized in the application. For example, you are already going to use the mutex for synchronization, and you already got lock contention by design in your application while the message-passing approach will not interrupt the I/O.

[zhengchengbin610]

Yes, I think so it depends on many factors; but because Enet is not thread-safe ,so I must use mutex lock to fix my problem, I use two thread only want to send faster, and my other mean is if the enet protocol designed high speed ,like Ack algorithm if the best now ?

[nxrighthere]

I don't think that you will benefit much from doing that, ENet is well multiplexed under the hood, and while stuff there works not asynchronously the transport is still doing its job very fast.

As for ACK and retransmission strategies, maybe modern protocols such as SCTP encapsulated into UDP are doing stuff better and more efficiently, but I've never measured it.

[zhengchengbin610]

@nxrighthere I have add mutex lock , it is work well when send and receive video stream, but it will appear a double free error when I call the release api . here is I wrapper the release api.
the gdb error as the attachment, look is the enet_peer_reset bug??

`
void apeman_reliable_udp_close(APEMAN_RELIABLE_UDP *udp)
{
pthread_mutex_lock(&(udp->hmutex));
if (NULL != udp->peer)
{
enet_peer_reset(udp->peer);
fprintf(stderr,"enet_peer_reset.........\n");
}
enet_host_destroy(udp->host);
pthread_mutex_unlock(&(udp->hmutex));

pthread_mutex_destroy(&(udp->hmutex));
enet_deinitialize();

}
`