Jump to content

  • Log In with Google      Sign In   
  • Create Account

Attaching a std::shared_ptr to a void* member


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
11 replies to this topic

#1 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 11:54 AM

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;
	}


Sponsor:

#2 fastcall22   Crossbones+   -  Reputation: 4461

Like
2Likes
Like

Posted 03 October 2013 - 12:32 PM

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?
c3RhdGljIGNoYXIgeW91cl9tb21bMVVMTCA8PCA2NF07CnNwcmludGYoeW91cl9tb20sICJpcyBmYXQiKTs=

#3 ApochPiQ   Moderators   -  Reputation: 16402

Like
2Likes
Like

Posted 03 October 2013 - 12:41 PM

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.

#4 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 12:59 PM

 

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?



#5 fastcall22   Crossbones+   -  Reputation: 4461

Like
3Likes
Like

Posted 03 October 2013 - 01:06 PM

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.

Edited by fastcall22, 03 October 2013 - 01:19 PM.

c3RhdGljIGNoYXIgeW91cl9tb21bMVVMTCA8PCA2NF07CnNwcmludGYoeW91cl9tb20sICJpcyBmYXQiKTs=

#6 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 01:22 PM

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;
			}

Edited by Tispe, 03 October 2013 - 01:30 PM.


#7 ApochPiQ   Moderators   -  Reputation: 16402

Like
2Likes
Like

Posted 03 October 2013 - 01:55 PM

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.



#8 fastcall22   Crossbones+   -  Reputation: 4461

Like
2Likes
Like

Posted 03 October 2013 - 02:02 PM

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

Edited by fastcall22, 03 October 2013 - 02:04 PM.

c3RhdGljIGNoYXIgeW91cl9tb21bMVVMTCA8PCA2NF07CnNwcmludGYoeW91cl9tb20sICJpcyBmYXQiKTs=

#9 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 02:11 PM

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);


#10 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 02:36 PM

 

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;
}


#11 Trienco   Crossbones+   -  Reputation: 2224

Like
0Likes
Like

Posted 03 October 2013 - 10:53 PM

Uhm... you realize that if you'd simply use a map<long long, PeerData> you would have exactly the same result? Why are you so hell bent on getting a shared_ptr involved when it's absolutely pointless? This feels a lot like "new toy syndrome".

 

In fact, it would even simplify things, as your PeerData only uses the default constructor and a map will automatically create a default constructed object if necessary.

 

PeerData* data = &AllPeers[ipandport]; will do exactly the same, without abusing the map as an artificial crutch way to keep your shared_ptr alive.


f@dzhttp://festini.device-zero.de

#12 Tispe   Members   -  Reputation: 1046

Like
0Likes
Like

Posted 03 October 2013 - 11:23 PM

Uhm... you realize that if you'd simply use a map<long long, PeerData> you would have exactly the same result? Why are you so hell bent on getting a shared_ptr involved when it's absolutely pointless? This feels a lot like "new toy syndrome".

 

In fact, it would even simplify things, as your PeerData only uses the default constructor and a map will automatically create a default constructed object if necessary.

 

PeerData* data = &AllPeers[ipandport]; will do exactly the same, without abusing the map as an artificial crutch way to keep your shared_ptr alive.

 

The std::map was intended to be implemented regardless. I want to be able to group peers in different groups later. For example if 3 players are on Team1, the shared_ptr would be placed in a new std::map called Team1. Then I can let Team1 chat with eachother by just traversing that container.

 

I also want an std::map that states which connected peers are "Authenticated", such that non-Authenticated peers can be filtered.

 

All of these containers would hold the same shared_ptr, but they will be used for different purposes.

 

Question, map<long long, PeerData>, would the PeerData object be on the stack or the heap?


Edited by Tispe, 03 October 2013 - 11:24 PM.





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS