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

[no sq] Server code cleanups / refactors #14438

Merged
merged 6 commits into from Mar 20, 2024
Merged

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Mar 6, 2024

depends on #14436, #14423
fixes #14005

To do

This PR is a Work in Progress / Ready for Review.

@sfan5 sfan5 added Bugfix 🐛 PRs that fix a bug Code quality labels Mar 6, 2024
@sfan5 sfan5 added the WIP The PR is still being worked on by its author and not ready yet. label Mar 10, 2024
@sfan5 sfan5 linked an issue Mar 10, 2024 that may be closed by this pull request
@lhofhansl
Copy link
Contributor

lhofhansl commented Mar 11, 2024

Can we try to just extract the fix that hangs MT after #14436 and get those in first?
(MinecClone2 is essentially unplayable... Hangs after a few minutes of play.)

I have reverted #14341 and #14436 locally.

@sfan5 sfan5 changed the title Server code cleanups / refactors [no sq] Server code cleanups / refactors Mar 12, 2024
@sfan5 sfan5 added Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) and removed WIP The PR is still being worked on by its author and not ready yet. labels Mar 12, 2024
@sfan5
Copy link
Member Author

sfan5 commented Mar 12, 2024

Commits squashed and no longer WIP.
I think the better way of getting the fix in is for someone to just review these PRs.

@sfan5 sfan5 force-pushed the servercpp branch 3 times, most recently from d5cc2f8 to 99f4a1a Compare March 14, 2024 09:56
@sfan5 sfan5 added Rebase needed The PR needs to be rebased by its author. and removed Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) labels Mar 17, 2024
@sfan5 sfan5 removed the Rebase needed The PR needs to be rebased by its author. label Mar 17, 2024
@sfan5 sfan5 marked this pull request as ready for review March 17, 2024 15:30
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compiles.
Singleplayer game showed no issues while testing for a while.
Unittests pass.
Code looks OK.

Found the following with valgrind upon shutdown (closing the window):

==15129== Thread 16 ConnectionSend:
==15129== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==15129==    at 0x5471A66: sendto (sendto.c:27)
==15129==    by 0x54EC38: UDPSocket::Send(Address const&, void const*, int) (in ./minetest)
==15129==    by 0x52C462: con::ConnectionSendThread::rawSend(con::BufferedPacket const*) (in ./minetest)
==15129==    by 0x531AF5: con::ConnectionSendThread::sendPackets(float, unsigned int) (in ./minetest)
==15129==    by 0x536BA7: con::ConnectionSendThread::run() (in ./minetest)
==15129==    by 0x6426D7: Thread::threadProc(Thread*) (in ./minetest)
==15129==    by 0x50F3252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==15129==    by 0x53DEAC2: start_thread (pthread_create.c:442)
==15129==    by 0x546FA03: clone (clone.S:100)
==15129==  Address 0x3d06746e is 318 bytes inside a block of size 512 alloc'd
==15129==    at 0x4849013: operator new(unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==15129==    by 0x522328: std::vector<unsigned char, std::allocator<unsigned char> >::_M_default_append(unsigned long) (in ./minetest)
==15129==    by 0x5180D4: con::makePacket(Address const&, SharedBuffer<unsigned char> const&, unsigned int, unsigned short, unsigned char) (in ./minetest)
==15129==    by 0x51DFA7: con::UDPPeer::processReliableSendCommand(std::shared_ptr<con::ConnectionCommand>&, unsigned int) (in ./minetest)
==15129==    by 0x51EACC: con::UDPPeer::PutReliableSendCommand(std::shared_ptr<con::ConnectionCommand>&, unsigned int) (in ./minetest)
==15129==    by 0x530294: con::ConnectionSendThread::processReliableCommand(std::shared_ptr<con::ConnectionCommand>&) (in ./minetest)
==15129==    by 0x536C92: con::ConnectionSendThread::run() (in ./minetest)
==15129==    by 0x6426D7: Thread::threadProc(Thread*) (in ./minetest)
==15129==    by 0x50F3252: ??? (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.30)
==15129==    by 0x53DEAC2: start_thread (pthread_create.c:442)
==15129==    by 0x546FA03: clone (clone.S:100)
==15129== 

but it doesn't appear that you changed this part. It might be an unrelated issue.

@sfan5 sfan5 merged commit 5727d74 into minetest:master Mar 20, 2024
13 checks passed
@sfan5 sfan5 deleted the servercpp branch March 20, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HTTP requests turn GET into PUT Avoid including client headers on server builds
3 participants