Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullifying pointer actually not good? #70

Merged
merged 1 commit into from
Feb 25, 2025
Merged

Conversation

JG-Adams
Copy link
Contributor

@JG-Adams JG-Adams commented Feb 25, 2025

This was confusing for my C++ brain. The addition I made here where you nullify pointer after freeing memory does absolutely nothing because the function takes a copy of a pointer. I'm used to thinking reference hence the mistake.

However, there are other confusion to this. I realized that actually nullifying pointer can cause program to crash rather than be stable. Take this example:

case ENET_EVENT_TYPE_DISCONNECT:
if constexpr (Server) {
deactivatePlayer(event.peer->incomingPeerID+1);
printf("Client%u has disconnected\n", event.peer->incomingPeerID+1);
if (event.peer) {
enet_peer_reset(event.peer); // Reset peer safely
//event.peer = nullptr; // Ensure we don't use it again. But this cause crashes???
}
BitWriter ms(packet_buffer, MESSAGE_SYSTEM_SERVERMESSAGE);
ms & "Player has disconnected!";
message_relay(event, ms, NET_CHANNEL_SYSTEM_SECURE);
}

One would want to send out message of a player leaving. So you would reset the peer thinking it should render it destroyed. My message_relay() would go through a loop and acquire the peer from the host. Not the event. and check with currentpeer->state == ENET_PEER_STATE_CONNECTED.

If you uncomment the nullification of event's pointer to peer this would crash. I don't understand how, but event's peer and host's peer somehow shared the same peer? So if you nullify one from event it applies to host too! This is very counter intuitive I would say. Are we not supposed to nullify them at all? I'm guessing that to be the case. However, you can nullify host and peer that exist outside of event.

Edit: I think it's shared probably because the service function for event may actually be updating the host with its own peer pointer. Shrug

This was confusing for my C++ brain. The addition I made here where you nullify pointer after freeing memory does absolutely nothing because the function takes a copy of a pointer. I'm used to thinking reference hence the mistake.

However, there are other confusion to this. I realized that nullifying pointer can actually cause program to crash rather than be stable. Take this example:

case ENET_EVENT_TYPE_DISCONNECT:
if constexpr (Server) {
     deactivatePlayer(event.peer->incomingPeerID+1);
     printf("Client%u has disconnected\n", event.peer->incomingPeerID+1);
    if (event.peer) {
   enet_peer_reset(event.peer);  // Reset peer safely
   //event.peer = nullptr;          // Ensure we don't use it again
                        }
                        BitWriter ms(packet_buffer, MESSAGE_SYSTEM_SERVERMESSAGE);
                        ms & "Player has disconnected!";
                        message_relay(event, ms, NET_CHANNEL_SYSTEM_SECURE);

                    }

One would want to send out message of a player leaving. So you would reset the peer thinking it should render it destroyed.
My message_relay() go through a loop and acquire the peer from the host. Not the event. and check with currentpeer->state == ENET_PEER_STATE_CONNECTED.

If you uncomment the nullification of pointer this would crash.
I don't understand how, but event's peer and host's peer somehow shared the same peer? So if you nullify one from event it applies to host too! This is very counter intuitive I would say. Are we not supposed to nullify them at all? I would think that to be the case.
@zpl-zak zpl-zak merged commit 13ed4c4 into zpl-c:master Feb 25, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants