Skip to content

Commit

Permalink
Fix linux daemon issue (#2572)
Browse files Browse the repository at this point in the history 
* [linux][mangosd] fix a memory leak in console CLI

* [mangosd][daemon] Don't start CLI when launched in daemon mode

* [linux][daemon] don't change dirs -> don't break relative paths
  • Loading branch information
@imranh2
imranh2 authored Apr 30, 2024
1 parent d328de0 commit 657d4f5
Show file tree
Hide file tree
Showing 5 changed files with 7 additions and 10 deletions.
3 changes: 3 additions & 0 deletions src/mangosd/CliRunnable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,5 +124,8 @@ void CliRunnable::operator()()
sWorld.QueueCliCommand(new CliCommandHolder(0, SEC_CONSOLE, nullptr, command.c_str(), &utf8print, &commandFinished));
}

This comment has been minimized.

Sign in to view

Copy link
@ReyDonovan

ReyDonovan Apr 30, 2024

Contributor 
    #ifndef WIN32
    free(command_str);
    #endif
}
#ifndef WIN32
free(command_str);
#endif
}
}
2 changes: 1 addition & 1 deletion src/mangosd/Main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ extern int main(int argc, char **argv)

// and run the 'Master'
// TODO: Why do we need this 'Master'? Can't all of this be in the Main as for Realmd?
return sMaster.Run();
return sMaster.Run(serviceDaemonMode);

// at sMaster return function exist with codes
// 0 - normal shutdown
Expand Down
4 changes: 2 additions & 2 deletions src/mangosd/Master.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ Master::~Master()
}

// Main function
int Master::Run()
int Master::Run(char serviceDaemonMode = '\0')
{
// worldd PID file creation
std::string pidfile = sConfig.GetStringDefault("PidFile", "");
Expand Down Expand Up @@ -210,7 +210,7 @@ int Master::Run()
#ifdef WIN32
if (sConfig.GetBoolDefault("Console.Enable", true) && (m_ServiceStatus == -1)/* need disable console in service mode*/)
#else
if (sConfig.GetBoolDefault("Console.Enable", true))
if (sConfig.GetBoolDefault("Console.Enable", true) && !serviceDaemonMode)
#endif
{
// Launch CliRunnable thread
Expand Down
2 changes: 1 addition & 1 deletion src/mangosd/Master.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Master
public:
Master();
~Master();
int Run();
int Run(char serviceDaemonMode);
static volatile uint32 m_masterLoopCounter;
static volatile bool m_handleSigvSignals;
static void SigvSignalHandler();
Expand Down
6 changes: 0 additions & 6 deletions src/shared/PosixDaemon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,9 @@ void startDaemon(uint32_t timeout)
exit(EXIT_FAILURE);
}

if ((chdir("/")) < 0)
{
exit(EXIT_FAILURE);
}

close(STDIN_FILENO);
close(STDOUT_FILENO);
close(STDERR_FILENO);

}

void stopDaemon()
Expand Down

1 comment on commit 657d4f5

@mserajnik
Copy link
Contributor

@mserajnik mserajnik commented on 657d4f5 Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

GCC and Clang builds fail:

/home/runner/work/core/core/src/mangosd/CliRunnable.cpp: In member function ‘void CliRunnable::operator()()’:
/home/runner/work/core/core/src/mangosd/CliRunnable.cpp:128:14: error: ‘command_str’ was not declared in this scope
  128 |         free(command_str);
      |              ^~~~~~~~~~~
[ 99%] Building CXX object src/mangosd/CMakeFiles/mangosd.dir/MaNGOSsoap.cpp.o
make[2]: *** [src/mangosd/CMakeFiles/mangosd.dir/build.make:76: src/mangosd/CMakeFiles/mangosd.dir/CliRunnable.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:556: src/mangosd/CMakeFiles/mangosd.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

The

#ifndef WIN32
free(command_str);
#endif

should be nested inside the if above it, as ReyDonovan already pointed out. The confusion here probably came from the conversation in the PR.

Ah, actually, the new if (s_canReadLine) introduced in 79c5f50 probably just caused the auto-merge to fuck up.

In addition, free() should also be used on Windows since that uses readline as well now.

Fixed in #2601.

Please sign in to comment.