draconisPW / PWMAngband

A free, multi-player roguelike dungeon exploration game based on Angband
35 stars 11 forks source link

free memory after death crush #592

Closed igroglaz closed 1 year ago

igroglaz commented 1 year ago

https://youtu.be/5C-XEgnIpf8

draconisPW commented 1 year ago

The video unfortunately doesn't show the value of p->body which would have helped greatly.

igroglaz commented 1 year ago

Maybe dump would help: https://stat.tangaria.com/chars/Varatorn-12450310022023.txt

igroglaz commented 1 year ago

And log:

100223 124449 New Level 3150ft at (0, -3) Ratings obj:365/mon:1323 100223 124449 Varatorn: You feel nervous about this place. 100223 124452 Varatorn: You sense the presence of creatures! 100223 124455 Varatorn: You sense no stairs. 100223 124458 Varatorn: You have found a trap. 100223 124458 Varatorn: You miss the ancient gold dragon. 100223 124458 Varatorn: You zap the ancient gold dragon for 64 damage. 100223 124458 Varatorn: You miss the ancient gold dragon. 100223 124458 Varatorn: You zap the ancient gold dragon for 58 damage. 100223 124458 Varatorn: You miss the ancient gold dragon. 100223 124458 Varatorn: The ancient gold dragon misses you. 100223 124458 Varatorn: The ancient gold dragon misses you. 100223 124458 Varatorn: The ancient gold dragon bites you for 46 damage. 100223 124459 Varatorn: The ancient gold dragon claws you for 20 damage. 100223 124459 Varatorn: The ancient gold dragon misses you. 100223 124459 Varatorn: The ancient gold dragon misses you. 100223 124459 Varatorn: You zap the ancient gold dragon for 80 damage. 100223 124459 Varatorn: You zap the ancient gold dragon for 52 damage. 100223 124459 Varatorn: You feel better. 100223 124459 Varatorn: You zap the ancient gold dragon for 60 damage. 100223 124459 Varatorn: You feel better. 100223 124459 Varatorn: You miss the ancient gold dragon. 100223 124459 Varatorn: You zap the ancient gold dragon for 56 damage. 100223 124459 Varatorn: The ancient gold dragon misses you. 100223 124459 Varatorn: The ancient gold dragon misses you. 100223 124459 Varatorn: The ancient gold dragon bites you for 44 damage. 100223 124459 Varatorn: You zap the ancient gold dragon for 58 damage. 100223 124459 Varatorn: You zap the ancient gold dragon for 76 damage. 100223 124459 Varatorn: You feel better. 100223 124459 Varatorn: You zap the ancient gold dragon for 70 damage. 100223 124459 Varatorn: You feel better. 100223 124459 Varatorn: You zap the ancient gold dragon for 121 damage. It was a good hit! 100223 124459 Varatorn: You feel better. 100223 124459 Varatorn: You miss the ancient gold dragon. 100223 124500 Varatorn: The ancient gold dragon breathes sound. 100223 124500 Varatorn: You resist the effect! 100223 124500 Varatorn: You zap the ancient gold dragon for 74 damage. 100223 124500 Varatorn: You zap the ancient gold dragon for 70 damage. 100223 124500 Varatorn: You miss the ancient gold dragon. 100223 124500 Varatorn: You zap the ancient gold dragon for 64 damage. 100223 124500 Varatorn: You zap the ancient gold dragon for 78 damage. 100223 124500 Varatorn: You feel better. 100223 124500 Varatorn: You miss the ancient gold dragon. 100223 124500 Varatorn: You zap the ancient gold dragon for 70 damage. 100223 124500 Varatorn: You miss the ancient gold dragon. 100223 124500 Varatorn: You zap the ancient gold dragon for 58 damage. 100223 124500 Varatorn: You feel better. 100223 124500 Varatorn: You zap the ancient gold dragon for 62 damage. 100223 124500 Varatorn: You feel better. 100223 124500 Varatorn: The ancient gold dragon misses you. 100223 124500 Varatorn: The ancient gold dragon misses you. 100223 124500 Varatorn: The ancient gold dragon bites you for 55 damage. 100223 124500 Varatorn: You zap the ancient gold dragon for 66 damage. 100223 124500 Varatorn: You feel better. 100223 124500 Varatorn: You zap the ancient gold dragon for 64 damage. 100223 124500 Varatorn: You zap the ancient gold dragon for 78 damage. 100223 124500 Varatorn: You zap the ancient gold dragon for 78 damage. 100223 124500 Varatorn: You feel better. 100223 124500 Varatorn: You zap the ancient gold dragon for 62 damage. 100223 124500 Varatorn: The ancient gold dragon flees in terror! 100223 124501 Varatorn: Your weapon calls beyond the grave! 100223 124501 Varatorn: Your weapon calls beyond the grave! 100223 124501 Varatorn: You drain the eye druj for 82 damage. It was a great hit! 100223 124501 Varatorn: You drain the eye druj for 49 damage. 100223 124501 Varatorn: You drain the eye druj for 46 damage. 100223 124501 Varatorn: You miss the eye druj. 100223 124501 Varatorn: You drain the eye druj for 47 damage. 100223 124501 Varatorn: The eye druj can move again. 100223 124501 Varatorn: The eye druj casts a bolt of nether. 100223 124501 Varatorn: You resist the effect! 100223 124501 Varatorn: You drain the eye druj for 52 damage. 100223 124501 Varatorn: You drain the eye druj for 46 damage. 100223 124501 Varatorn: You drain the eye druj for 57 damage. 100223 124501 Varatorn: You miss the eye druj. 100223 124501 Varatorn: You drain the eye druj for 87 damage. It was a good hit! 100223 124501 Varatorn: The ancient gold dragon is no longer afraid. 100223 124502 Varatorn: The eye druj casts a ball of nether. 100223 124502 Varatorn: LOW HITPOINT WARNING! 100223 124502 Varatorn: You resist the effect! 100223 124502 Varatorn: The ancient gold dragon dies. 100223 124502 Varatorn: You drain the eye druj for 52 damage. 100223 124502 Varatorn: You drain the eye druj for 51 damage. 100223 124502 Varatorn: You drain the eye druj for 54 damage. 100223 124502 Varatorn: You drain the eye druj for 49 damage. 100223 124502 Varatorn: You drain the eye druj for 55 damage. 100223 124503 Varatorn: The eye druj smashes you with psionic energy. 100223 124503 Varatorn: You resist the effect! 100223 124503 Varatorn: You resist the effect! 100223 124503 Varatorn: You resist the effect! 100223 124503 Varatorn: You resist the effect! 100223 124503 Varatorn: You die. 100223 124503 Character dump successful (server). 100223 124503 Character dump successful (client). 100223 124503 Varatorn: You re-arrange your quiver. 100223 124503 Varatorn: You re-arrange your quiver. 100223 124503 Varatorn: You re-arrange your quiver. 100223 124503 Varatorn: You re-arrange your quiver. 100223 124503 Varatorn: You have been killed by an eye druj. 100223 124504 Goodbye Varatorn=Varatorn@HOME-PC ("Killed by an eye druj")

draconisPW commented 1 year ago

Need to check if this comes from Vampire race or it's just some random memory corruption like the one from #591 because I don't see how slots pointer could provoke a crash...

igroglaz commented 1 year ago

good point. but #591 was client crush, while this issue is server crush.. but who knows, maybe there is a connection

draconisPW commented 1 year ago

For me it looks like another instance of double free somewhere. I will consolidate the code by adding p = NULL lines after every mem_free(p).

draconisPW commented 1 year ago

Rules:

We need to enforce these rules to minimize the chance of such crashes as described in the issue description to occur.

draconisPW commented 1 year ago

Server side work done.

draconisPW commented 1 year ago

Client side work done.

draconisPW commented 1 year ago

Common code updated (without string_free).

igroglaz commented 1 year ago

image

image

image

image

image

image

image

it's crush from today.. all updates ported except 2 last ones (88a70ad92725d6279a69c4288dfd8f28e5aa056b)

draconisPW commented 1 year ago

Seems like memory is erased from something else, as there is nothing wrong with the screens above. Only explanation is that the string s is acceding deleted memory.

igroglaz commented 1 year ago

Serakin XXVII died.. but after death log says that he re-arranged pack and quiver. This message shouldn't appear in log after char death

igroglaz commented 1 year ago

image

image

image

image

image

image

image

image

draconisPW commented 1 year ago

Input handler seems to be called after character leaves before it is removed by the "leave" function. I'll look into that.

draconisPW commented 1 year ago

Problem: method Contact is not called when I disconnect a character, it's the input handler that is removed. I fear the problem comes from Tangaria loading too much stuff.

At stage 2 if the client takes too long the server will listen on an empty socket and since it's asynchronous it will respond with EWOULDBLOCK.

draconisPW commented 1 year ago

Handled EWOULDBLOCK in Contact. If other errors still occur (errno != EWOULDBLOCK ) just disable the "quit" line in Tangaria so at least the server doesn't quit and just refuses the connection from client.

igroglaz commented 1 year ago

Back to original crush (p->body....):

image

image

image

image

image

image

aaand p->body... :

image

image

image

image

image

image

and some internals:

image

image

image

image

image

image

image

image

hope it's enough :)

igroglaz commented 1 year ago

updated some screenshots in previous post

draconisPW commented 1 year ago

body.slots pointer seems to be valid, this is really confusing. Maybe it's because "obj" is pointing to a valid object and shouldn't be freed... I'll take a look.

draconisPW commented 1 year ago

Unfortunately the most vital information (value of p->body.slots) is missing from the post above, which makes it impossible to debug the issue. I wont be able to "guess" the reason here... Looking at V code, I suspect it's because I condensed the allocation with a "memcpy" when I shouldn't have, so I'll revert the code to what is done in V. If the issue still happens after this fix, we'll have to check in detail the value of p->body.slots (it's an array so it has to be expanded since the debugger only shows the first item).

draconisPW commented 1 year ago

Ofc I'm an idiot.

p->body.slots[i].name must me allocated and freed separately, that's obviously the reason why it crashes... Fixing this moronic bug...