Attaching a std::shared_ptr to a void* member

Started by
10 comments, last by Tispe 10 years, 6 months ago

Hi

I am having trouble attaching a std::shared_ptr to a void* member inside a struct for the Enet library. The netevent.peer->data member is of type void*, I want to be able to create a shared_ptr and set it to netevent.peer->data. When I recieve packets I can cast netevent.peer->data back to a shared_ptr and get the members out. But all the tricks i've tried so far has crashed the application.

Any input?


case ENET_EVENT_TYPE_CONNECT:
	{
		char buffer[64];
		in_addr addr;
		long long ipandport = (netevent.peer->address.host << 16) | netevent.peer->address.port;		//Unique ID
		std::shared_ptr<PeerData> spNewPeer(new PeerData);
		spNewPeer->ipandport = ipandport;
		spNewPeer->peer = netevent.peer;
		addr.S_un.S_addr = netevent.peer->address.host;
		sprintf_s(buffer, "%s:%d", inet_ntoa(addr), netevent.peer->address.port);
		printf("A new client connected from %s.\n", buffer);
		spNewPeer->sipandport = string(buffer);
		netevent.peer->data = &(*spNewPeer);						//assigning, might be bad....
		cout << "Registered user: " << ipandport << endl;
		break;
	}
case ENET_EVENT_TYPE_RECEIVE:
	{
		std::shared_ptr<PeerData> spPeer(netevent.peer->data);
		cout << "A packet of length " << netevent.packet->dataLength 
			<< " containing " << netevent.packet->data 
			<< " was received from " << spPeer->sipandport			//causes compile errors, cannot convert parameter 1 from 'void *' to 'PeerData *'
			<< " on channel" << netevent.channelID << "." << endl;

		if(netevent.packet != NULL){
			enet_packet_destroy (netevent.packet);
		}
		break;
	}
Advertisement

I want to be able to create a shared_ptr and set it to netevent.peer->data. When I recieve packets I can cast netevent.peer->data back to a shared_ptr and get the members out. But all the tricks i've tried so far has crashed the application.


Okay, there are several major problems here:
First, you cannot send a pointer to an object. As an example, imagine I sold an item to you. Instead of packaging the item in a box and shipping it, I mail you the location of where the item is in my house. What good does knowing where the item is in my house help you? This is essentially sending a pointer to an object. Instead, you'll need to package the object (via serialization) and send it out. If you need to refer to this object over the network later, use a unique identifier (such as an integer) to refer to the object on both machines.

Second, you cannot send a shared pointer as it is over the network, for the same reasons given above. A shared pointer is basically an integer keeping track of the number of references, and the object its pointing to. If you were to somehow send both the object and the reference count, how do the machines synchronize the reference count? Why does one machine care about the other's memory management?
Not to mention that a shared_ptr to void* doesn't make much sense. A void* is just a pointer, not the actual data pointed to by the pointer, and it's trivial to copy/share the pointer itself. shared_ptr is for holding on to a resource with indeterminate lifetime, typically managed via a destructor and RAII.

Sounds like what you want is to have a void* point into a buffer that is managed by some other logic, which is a totally different beast, and probably not actually compatible with the library you're using.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

I want to be able to create a shared_ptr and set it to netevent.peer->data. When I recieve packets I can cast netevent.peer->data back to a shared_ptr and get the members out. But all the tricks i've tried so far has crashed the application.


Okay, there are several major problems here:
First, you cannot send a pointer to an object. As an example, imagine I sold an item to you. Instead of packaging the item in a box and shipping it, I mail you the location of where the item is in my house. What good does knowing where the item is in my house help you? This is essentially sending a pointer to an object. Instead, you'll need to package the object (via serialization) and send it out. If you need to refer to this object over the network later, use a unique identifier (such as an integer) to refer to the object on both machines.

Second, you cannot send a shared pointer as it is over the network, for the same reasons given above. A shared pointer is basically an integer keeping track of the number of references, and the object its pointing to. If you were to somehow send both the object and the reference count, how do the machines synchronize the reference count? Why does one machine care about the other's memory management?

I think you need to reread my original post. The peer->data member is never sent anywhere. It is just a pointer in the peer object where the application can put data.

Not to mention that a shared_ptr to void* doesn't make much sense. A void* is just a pointer, not the actual data pointed to by the pointer, and it's trivial to copy/share the pointer itself. shared_ptr is for holding on to a resource with indeterminate lifetime, typically managed via a destructor and RAII.

Sounds like what you want is to have a void* point into a buffer that is managed by some other logic, which is a totally different beast, and probably not actually compatible with the library you're using.

What about having the void* point to a shared_ptr? peer->data->spMyPointer->MyMember? Will the shared_ptr keep reference count and not delete itself for this method?

The peer->data member is never sent anywhere. It is just a pointer in the peer object where the application can put data.


Why are you storing things in a packet and not sending it?

Taking a closer look, the PeerData is deleted when the case scope ends, because the shared pointer that owns it falls out of scope and there no other shared pointers referencing that data. Attempting to read from peer->data later segfaults, since the data has already been freed.

Added an std::map to own the shared_ptr but on the second packet the application crashes. It works once, then crash .... Getting an access violation.

The statement std::shared_ptr<PeerData> spPeer((PeerData*)netevent.peer->data); results in an spPeer with unvalid members.


std::map<long long, std::shared_ptr<PeerData>> AllPeers;
bool loop = true;
while(loop){
	ENetEvent netevent;
	/* Wait up to 1000 milliseconds for an event. */
	if(enet_host_service (server, &netevent, 10)){
		switch (netevent.type)    {    
		case ENET_EVENT_TYPE_CONNECT:
			{
				char buffer[64];
				in_addr addr;
				long long ipandport = netevent.peer->address.host;
				ipandport = (ipandport << 16) | netevent.peer->address.port;		//Unique ID
				std::shared_ptr<PeerData> spNewPeer(new PeerData);
				spNewPeer->ipandport = ipandport;
				spNewPeer->peer = netevent.peer;
				addr.S_un.S_addr = netevent.peer->address.host;
				sprintf_s(buffer, "%s:%d", inet_ntoa(addr), netevent.peer->address.port);
				printf("A new client connected from %s.\n", buffer);
				spNewPeer->sipandport = string(buffer);
				netevent.peer->data = spNewPeer.get();								//assigning, might be bad....
				cout << "Registered user: " << ipandport << endl;
				AllPeers[ipandport] = spNewPeer;
				break;
			}
		case ENET_EVENT_TYPE_RECEIVE:
			{
				std::shared_ptr<PeerData> spPeer((PeerData*)netevent.peer->data);
				cout << "A packet of length " << netevent.packet->dataLength 
					<< " containing " << netevent.packet->data 
					<< " was received from " << spPeer->sipandport			//causes compile errors, cannot convert parameter 1 from 'void *' to 'PeerData *'
					<< " on channel" << netevent.channelID << "." << endl;

				if(netevent.packet != NULL){
					enet_packet_destroy (netevent.packet);
				}
				break;
			}

You should take a step or three back.

First, understand the purpose and intended use cases of shared_ptr. What you're doing here makes zero sense, and it's clear you're not familiar with how RAII works or how object lifetimes work or even how shared memory management works.

Second, think carefully about the way you use anything related to void pointers. They are dangerous, tricky, and easy to cause bugs with. Avoid them if at all possible.

Third, please at least make an effort to describe the problem you're trying to solve instead of just posting code that can't possibly be used to reproduce your problem and expecting people to magically figure it out.

Wielder of the Sacred Wands
[Work - ArenaNet] [Epoch Language] [Scribblings]

std::shared_ptr<PeerData> spPeer((PeerData*)netevent.peer->data);


When you construct a shared pointer from a raw pointer, the shared pointer assumes ownership of the pointer. See the sample code below:
#include <memory>

int main(int,char*[]) {
    typedef std::shared_ptr<int> SharedInt;
    void* ptr;
    
    SharedInt a(new int);
    /*1*/ {
        ptr = &(*a);
    }
    
    /*2*/ {
        SharedInt b( (int*)ptr );
        *b = 5;
    } // b is destroyed, ptr is deleted, and a is dangling
} // Crash once a is destroyed

You should take a step or three back.

First, understand the purpose and intended use cases of shared_ptr. What you're doing here makes zero sense, and it's clear you're not familiar with how RAII works or how object lifetimes work or even how shared memory management works.

Second, think carefully about the way you use anything related to void pointers. They are dangerous, tricky, and easy to cause bugs with. Avoid them if at all possible.

Third, please at least make an effort to describe the problem you're trying to solve instead of just posting code that can't possibly be used to reproduce your problem and expecting people to magically figure it out.

I wanted to do the same as described in this tutorial (http://www.linuxjournal.com/content/network-programming-enet?page=0,1) but using a shared_ptr instead of raw pointer. The tutorial uses malloc() and free() on the peer->data. I just wanted to avoid leaks and use shared_ptr.


case ENET_EVENT_TYPE_RECEIVE:
	if (event.peer->data == NULL) {
		event.peer->data = malloc(strlen((char*) event.packet->data)+1);
		strcpy((char*) event.peer->data, (char*) event.packet->data);

		sprintf(buffer, "%s has connected\n", (char*) event.packet->data);

case ENET_EVENT_TYPE_DISCONNECT:
	sprintf(buffer, "%s has disconnected.", (char*) event.peer->data);
	packet = enet_packet_create(buffer, strlen(buffer)+1, 0);
	enet_host_broadcast(server, 1, packet);
	free(event.peer->data);


std::shared_ptr<PeerData> spPeer((PeerData*)netevent.peer->data);

When you construct a shared pointer from a raw pointer, the shared pointer assumes ownership of the pointer. See the sample code below:

#include <memory>

int main(int,char*[]) {
    typedef std::shared_ptr<int> SharedInt;
    void* ptr;
    
    SharedInt a(new int);
    /*1*/ {
        ptr = &(*a);
    }
    
    /*2*/ {
        SharedInt b( (int*)ptr );
        *b = 5;
    } // b is destroyed, ptr is deleted, and a is dangling
} // Crash once a is destroyed

Thanks, got it working now. Here is all the code, which is now working. If you got any comments/improvements let me know ;)


struct PeerData {
	DWORD Status;
	long long ipandport;
	ENetPeer* peer;
	string sipandport;
};

int Server(){
	if (enet_initialize () != 0){
		cout << "An error occurred while initializing ENet.\n";
		enet_deinitialize();
		return EXIT_FAILURE;
	}

	ENetAddress address;
	ENetHost * server;
	/* Bind the server to the default localhost.     */
	/* A specific host address can be specified by   */
	/* enet_address_set_host (& address, "x.x.x.x"); */
	address.host = ENET_HOST_ANY;
	/* Bind the server to port 1234. */
	address.port = 1234;
	server = enet_host_create (& address /* the address to bind the server host to */,                              
		32      /* allow up to 32 clients and/or outgoing connections */,                              
		2      /* allow up to 2 channels to be used, 0 and 1 */,                              
		0      /* assume any amount of incoming bandwidth */,                              
		0      /* assume any amount of outgoing bandwidth */);
	if (server == NULL){    
		cout << "An error occurred while trying to create an ENet server host.\n";    
		return EXIT_FAILURE;
	}

	//Main Loop
	std::map<long long, std::shared_ptr<PeerData>> AllPeers;
	bool loop = true;
	while(loop){
		ENetEvent netevent;
		/* Wait up to 1000 milliseconds for an event. */
		if(enet_host_service (server, &netevent, 10)){
			switch (netevent.type){    
			case ENET_EVENT_TYPE_CONNECT:
				{
					char buffer[64];
					in_addr addr;
					long long ipandport = netevent.peer->address.host;
					ipandport = (ipandport << 16) | netevent.peer->address.port;		//Unique ID
					std::shared_ptr<PeerData> spNewPeer(new PeerData);
					spNewPeer->ipandport = ipandport;
					spNewPeer->peer = netevent.peer;
					addr.S_un.S_addr = netevent.peer->address.host;
					sprintf_s(buffer, "%s:%d", inet_ntoa(addr), netevent.peer->address.port);
					printf("A new client connected from %s.\n", buffer);
					spNewPeer->sipandport = string(buffer);
					netevent.peer->data = spNewPeer.get();
					cout << "Registered user: " << ipandport << endl;
					AllPeers[ipandport] = spNewPeer;
					break;
				}
			case ENET_EVENT_TYPE_RECEIVE:
				{
					cout << "A packet of length " << netevent.packet->dataLength 
						<< " containing " << netevent.packet->data 
						<< " was received from " << ((PeerData*)netevent.peer->data)->sipandport
						<< " on channel " << int(netevent.channelID) << "." << endl;

					if(netevent.packet != NULL){
						enet_packet_destroy (netevent.packet);
					}
					break;
				}
			case ENET_EVENT_TYPE_DISCONNECT:        
				printf("%s disconected.\n", ((PeerData*)netevent.peer->data)->sipandport.c_str());        
				AllPeers.erase(((PeerData*)netevent.peer->data)->ipandport);  
				netevent.peer->data = NULL;    
			}
		}
	}

	//Reset all peers

	enet_host_destroy(server);
	enet_deinitialize();
	return 0;
}

This topic is closed to new replies.

Advertisement