From 1b4da37f0a8cb8b32ebdb2c6a5227c7f89f43ee3 Mon Sep 17 00:00:00 2001 From: Karol Grydziuszko Date: Fri, 26 Nov 2021 12:41:07 +0100 Subject: [PATCH 1/3] Code refactor of CmdExecutor: got rid of detaching thread equal fixing managing HilogBuffer.Added ability to timeout on polling and cleaning finished connected client meanwhile. Signed-off-by: Karol Grydziuszko Change-Id: I8b56bef05549a57fbbfa4915a7f9b0c931388ee9 --- frameworks/native/BUILD.gn | 1 + frameworks/native/include/linux_utils.h | 32 ++++ .../native/include/seq_packet_socket_server.h | 9 +- frameworks/native/include/socket_server.h | 2 + frameworks/native/linux_utils.cpp | 118 ++++++++++++++ .../native/seq_packet_socket_server.cpp | 31 ++-- frameworks/native/socket_server.cpp | 9 + services/hilogd/cmd_executor.cpp | 154 +++++++++++++++--- services/hilogd/include/cmd_executor.h | 27 ++- services/hilogd/main.cpp | 2 +- 10 files changed, 336 insertions(+), 49 deletions(-) create mode 100644 frameworks/native/include/linux_utils.h create mode 100644 frameworks/native/linux_utils.cpp diff --git a/frameworks/native/BUILD.gn b/frameworks/native/BUILD.gn index 28a55a7..a5f41f8 100644 --- a/frameworks/native/BUILD.gn +++ b/frameworks/native/BUILD.gn @@ -66,6 +66,7 @@ ohos_shared_library("libhilogutil") { "dgram_socket_client.cpp", "dgram_socket_server.cpp", "format.cpp", + "linux_utils.cpp", "seq_packet_socket_client.cpp", "seq_packet_socket_server.cpp", "socket.cpp", diff --git a/frameworks/native/include/linux_utils.h b/frameworks/native/include/linux_utils.h new file mode 100644 index 0000000..446fa33 --- /dev/null +++ b/frameworks/native/include/linux_utils.h @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2021 Huawei Device Co., Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef HIVIEWDFX_LINUX_UTILS_H +#define HIVIEWDFX_LINUX_UTILS_H + +#include + +namespace OHOS { +namespace HiviewDFX { +namespace Utils { +std::string_view ListenErrorToStr(int error); +std::string_view AcceptErrorToStr(int error); +std::string_view ChmodErrorToStr(int error); +std::string_view PollErrorToStr(int error); +} // Utils +} // HiviewDFX +} // OHOS + +#endif // HIVIEWDFX_LINUX_UTILS_H diff --git a/frameworks/native/include/seq_packet_socket_server.h b/frameworks/native/include/seq_packet_socket_server.h index c214d6a..e9d0ce2 100644 --- a/frameworks/native/include/seq_packet_socket_server.h +++ b/frameworks/native/include/seq_packet_socket_server.h @@ -17,19 +17,22 @@ #define SEQ_PACKET_SOCKET_SERVER_H #include "socket_server.h" +#include +#include namespace OHOS { namespace HiviewDFX { -typedef int (*AcceptingHandler)(std::unique_ptr); class SeqPacketSocketServer : public SocketServer { public: + using AcceptingHandler = std::function)>; + SeqPacketSocketServer(const std::string& serverPath, unsigned int maxListenNumber) : SocketServer(serverPath, SOCK_SEQPACKET), maxListenNumber(maxListenNumber) {} ~SeqPacketSocketServer() = default; - int AcceptConnection(AcceptingHandler func); + int StartAcceptingConnection(AcceptingHandler onAccepted); private: unsigned int maxListenNumber; - int AcceptingThread(AcceptingHandler func); + int AcceptingLoop(AcceptingHandler func); }; } // namespace HiviewDFX } // namespace OHOS diff --git a/frameworks/native/include/socket_server.h b/frameworks/native/include/socket_server.h index 2b98a34..f668198 100644 --- a/frameworks/native/include/socket_server.h +++ b/frameworks/native/include/socket_server.h @@ -21,6 +21,7 @@ #include #include #include +#include #include "hilog_common.h" #include "socket_client.h" @@ -35,6 +36,7 @@ public: int Recv(void *buffer, unsigned int bufferLen, int flags = MSG_PEEK); int RecvMsg(struct msghdr *hdr, int flags = 0); int Listen(unsigned int backlog); + int Poll(short inEvent, short& outEvent, const std::chrono::milliseconds& timeout); int Accept(); private: int socketHandler; diff --git a/frameworks/native/linux_utils.cpp b/frameworks/native/linux_utils.cpp new file mode 100644 index 0000000..bad09bf --- /dev/null +++ b/frameworks/native/linux_utils.cpp @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2021 Huawei Device Co., Ltd. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "linux_utils.h" +#include +#include +using namespace std::literals; + +namespace OHOS { +namespace HiviewDFX { +namespace Utils { +std::string_view ListenErrorToStr(int error) +{ + static const std::unordered_map ERRORS { + // explanations taken from https://man7.org/linux/man-pages/man2/listen.2.html + {EADDRINUSE, "Another socket is already listening on the same port or " + "The socket referred to by sockfd " + "had not previously been bound to an address and, upon " + "attempting to bind it to an ephemeral port, it was " + "determined that all port numbers in the ephemeral port " + "range are currently in use."sv}, + {EBADF, "The argument sockfd is not a valid file descriptor."sv}, + {ENOTSOCK, "The file descriptor sockfd does not refer to a socket."sv}, + {EOPNOTSUPP, "The socket is not of a type that supports the listen() operation."sv}, + {0, "Success"sv} + }; + auto it = ERRORS.find(error); + return it != ERRORS.end() ? it->second : "Unknown error"sv; +} + +std::string_view AcceptErrorToStr(int error) +{ + static const std::unordered_map ERRORS { + // explanations taken from https://man7.org/linux/man-pages/man2/accept.2.html + {EOPNOTSUPP, "The socket is not of a type that supports the listen() operation."sv}, + {EAGAIN, "The socket is marked nonblocking and no connections are present to be accepted."sv}, + {EWOULDBLOCK, "The socket is marked nonblocking and no connections are present to be accepted."sv}, + {EBADF, "sockfd is not an open file descriptor."sv}, + {ECONNABORTED, "A connection has been aborted."sv}, + {EFAULT, "The addr argument is not in a writable part of the user address space."sv}, + {EINTR, "The system call was interrupted by a signal that was caught before a valid connection arrived."sv}, + {EINVAL, "Socket is not listening for connections, or addrlen is invalid (e.g., is negative)."sv}, + {EMFILE, "The per-process limit on the number of open file descriptors has been reached."sv}, + {ENFILE, "The system-wide limit on the total number of open files has been reached."sv}, + {ENOBUFS, "Not enough free buffers. This often means that the memorsv " + "allocation is limited by the socket buffer limits, not by " + "the system memory."sv}, + {ENOMEM, "Not enough free memory. This often means that the memory " + "allocation is limited by the socket buffer limits, not by " + "the system memory."sv}, + {ENOTSOCK, "The file descriptor sockfd does not refer to a socket."sv}, + {EOPNOTSUPP, "The referenced socket is not of type SOCK_STREAM."sv}, + {EPERM, "Firewall rules forbid connection."sv}, + {EPROTO, "Protocol error."sv} + }; + auto it = ERRORS.find(error); + return it != ERRORS.end() ? it->second : "Unknown error"sv; +} + +std::string_view ChmodErrorToStr(int error) +{ + static const std::unordered_map ERRORS { + // explanations taken from https://man7.org/linux/man-pages/man2/chmod.2.html + {EACCES, "Search permission is denied on a component of the path prefix."sv}, + {EBADF, "(fchmod()) The file descriptor fd is not valid."sv}, + {EBADF, "(fchmodat()) pathname is relative but dirfd is neither AT_FDCWD nor a valid file descriptor."sv}, + {EFAULT, "pathname points outside your accessible address space."sv}, + {EINVAL, "(fchmodat()) Invalid flag specified in flags."sv}, + {EIO, "An I/O error occurred."sv}, + {ELOOP, "Too many symbolic links were encountered in resolving pathname."sv}, + {ENAMETOOLONG, "pathname is too long."sv}, + {ENOENT, "The file does not exist."sv}, + {ENOMEM, "Insufficient kernel memory was available."sv}, + {ENOTDIR, "A component of the path prefix is not a directory."sv}, + {ENOTDIR, "(fchmodat()) pathname is relative and dirfd is a file " + "descriptor referring to a file other than a directory."sv}, + {ENOTSUP, "(fchmodat()) flags specified AT_SYMLINK_NOFOLLOW, which is not supported."sv}, + {EPERM, "The effective UID does not match the owner of the file, " + "and the process is not privileged (Linux: it does not have " + "the CAP_FOWNER capability)."sv}, + {EPERM, "The file is marked immutable or append-only. (See ioctl_iflags(2).)"sv}, + {EROFS, "The named file resides on a read-only filesystem."sv} + }; + auto it = ERRORS.find(error); + return it != ERRORS.end() ? it->second : "Unknown error"sv; +} + +std::string_view PollErrorToStr(int error) +{ + static const std::unordered_map ERRORS { + // explanations taken from https://man7.org/linux/man-pages/man2/poll.2.html + {EFAULT, "fds points outside the process's accessible address space. " + "The array given as argument was not contained in the " + "calling program's address space."sv}, + {EINTR, "A signal occurred before any requested event;"sv}, + {EINVAL, "The nfds value exceeds the RLIMIT_NOFILE value."sv}, + {EINVAL, "(ppoll()) The timeout value expressed in *ip is invalid (negative)."sv}, + {ENOMEM, "Unable to allocate memory for kernel data structures."sv}, + }; + auto it = ERRORS.find(error); + return it != ERRORS.end() ? it->second : "Unknown error"sv; +} + +} // Utils +} // HiviewDFX +} // OHOS diff --git a/frameworks/native/seq_packet_socket_server.cpp b/frameworks/native/seq_packet_socket_server.cpp index c8665b2..77f1d03 100644 --- a/frameworks/native/seq_packet_socket_server.cpp +++ b/frameworks/native/seq_packet_socket_server.cpp @@ -17,34 +17,39 @@ #include #include +#include "linux_utils.h" namespace OHOS { namespace HiviewDFX { -int SeqPacketSocketServer::AcceptConnection(AcceptingHandler func) +int SeqPacketSocketServer::StartAcceptingConnection(AcceptingHandler onAccepted) { - int ret = Listen(maxListenNumber); - if (ret < 0) { + int listeningStatus = Listen(maxListenNumber); + if (listeningStatus < 0) { #ifdef DEBUG - std::cout << "Socket listen failed: " << ret << std::endl; + std::cerr << "Socket listen failed: " << listeningStatus << "\n"; + std::cerr << Utils::ListenErrorToStr(listeningStatus) << "\n"; #endif - return ret; + return listeningStatus; } - AcceptingThread(func); - - return ret; + return AcceptingLoop(onAccepted); } -int SeqPacketSocketServer::AcceptingThread(AcceptingHandler func) +int SeqPacketSocketServer::AcceptingLoop(AcceptingHandler func) { - int ret = 0; - while ((ret = Accept()) > 0) { + int acceptedSockedFd = 0; + while ((acceptedSockedFd = Accept()) > 0) { std::unique_ptr handler = std::make_unique(SOCK_SEQPACKET); - handler->setHandler(ret); + handler->setHandler(acceptedSockedFd); func(std::move(handler)); } + int acceptError = errno; +#ifdef DEBUG + std::cerr << "Socket accept failed: " << acceptError << "\n"; + std::cerr << Utils::AccessErrorToStr(acceptError) << "\n"; +#endif - return ret; + return acceptError; } } // namespace HiviewDFX } // namespace OHOS diff --git a/frameworks/native/socket_server.cpp b/frameworks/native/socket_server.cpp index 9ffb16e..b2e0a3f 100644 --- a/frameworks/native/socket_server.cpp +++ b/frameworks/native/socket_server.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include "socket_server_adapter.h" @@ -89,6 +90,14 @@ int SocketServer::Listen(unsigned int backlog) return listen(socketHandler, backlog); } +int SocketServer::Poll(short inEvent, short& outEvent, const std::chrono::milliseconds& timeout) +{ + pollfd info {socketHandler, inEvent, outEvent}; + int result = poll(&info, 1, timeout.count()); + outEvent = info.revents; + return result; +} + int SocketServer::Accept() { socklen_t addressSize = sizeof(serverAddr); diff --git a/services/hilogd/cmd_executor.cpp b/services/hilogd/cmd_executor.cpp index 86946fa..5a0ba4e 100644 --- a/services/hilogd/cmd_executor.cpp +++ b/services/hilogd/cmd_executor.cpp @@ -15,6 +15,7 @@ #include "cmd_executor.h" #include "log_querier.h" #include "seq_packet_socket_server.h" +#include "linux_utils.h" #include #include @@ -22,51 +23,154 @@ #include #include #include +#include +#include +#include +#include namespace OHOS { namespace HiviewDFX { const int MAX_WRITE_LOG_TASK = 100; -using namespace std; -HilogBuffer* CmdExecutor::hilogBuffer = nullptr; +CmdExecutor::CmdExecutor(HilogBuffer* buffer) +{ + m_hilogBuffer = buffer; +} -void LogQuerierMonitor(std::unique_ptr handler) +CmdExecutor::~CmdExecutor() { - prctl(PR_SET_NAME, "hilogd.query"); - std::shared_ptr logQuerier = std::make_shared(std::move(handler), - CmdExecutor::getHilogBuffer()); - logQuerier->LogQuerierThreadFunc(logQuerier); + std::lock_guard lg(m_clientAccess); + for (auto& client : m_clients) { + client->m_stopThread.store(true); + } + for (auto& client : m_clients) { + if (client->m_clientThread.joinable()) { + client->m_clientThread.join(); + } + } } -int CmdExecutorThreadFunc(std::unique_ptr handler) +void CmdExecutor::MainLoop() { - std::thread logQuerierMonitorThread(LogQuerierMonitor, std::move(handler)); - logQuerierMonitorThread.detach(); - return 0; + SeqPacketSocketServer cmdServer(CONTROL_SOCKET_NAME, MAX_WRITE_LOG_TASK); + if (cmdServer.Init() < 0) { + std::cerr << "Failed to init control socket ! \n"; + return; + } + if (chmod(CONTROL_SOCKET, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) < 0) { + int chmodError = errno; + std::cerr << "chmod control socket failed ! Error: " << chmodError << "\n"; + std::cerr << Utils::ChmodErrorToStr(chmodError) << "\n"; + } + std::cout << "Begin to cmd accept !\n"; + int listeningStatus = cmdServer.Listen(MAX_WRITE_LOG_TASK); + if (listeningStatus < 0) { + std::cerr << "Socket listen failed: " << listeningStatus << "\n"; + std::cerr << Utils::ListenErrorToStr(listeningStatus) << "\n"; + return; + } + std::cout << "Server started to listen !\n"; + + using namespace std::chrono_literals; + for(;;) { + const auto maxtime = 3000ms; + short outEvent = 0; + auto pollResult = cmdServer.Poll(POLLIN, outEvent, maxtime); + if (pollResult == 0) { + // std::cout << "---------Timeout---------\n"; + CleanFinishedClients(); + continue; + } + else if (pollResult < 0) { + int pollError = errno; + std::cerr << "Socket polling error: " << pollError << "\n"; + std::cerr << Utils::ChmodErrorToStr(pollError) << "\n"; + break; + } + else if (pollResult != 1 || outEvent != POLLIN) { + std::cerr << "Wrong poll result data." + " Result: " << pollResult << + " OutEvent: " << outEvent << "\n"; + break; + } + + int acceptResult = cmdServer.Accept(); + if (acceptResult > 0) { + // std::cout << "---------New Connection---------\n"; + int acceptedSockedFd = acceptResult; + std::unique_ptr handler = std::make_unique(SOCK_SEQPACKET); + handler->setHandler(acceptedSockedFd); + OnAcceptedConnection(std::move(handler)); + } + else { + int acceptError = errno; + std::cerr << "Socket accept failed: " << acceptError << "\n"; + std::cerr << Utils::AcceptErrorToStr(acceptError) << "\n"; + break; + } + } } -CmdExecutor::CmdExecutor(HilogBuffer* buffer) +void CmdExecutor::OnAcceptedConnection(std::unique_ptr handler) { - hilogBuffer = buffer; + std::lock_guard lg(m_clientAccess); + auto newVal = std::unique_ptr(new ClientThread{ + std::thread(&CmdExecutor::ClientEventLoop, this, std::move(handler)), + std::atomic(false) + }); + m_clients.push_back(std::move(newVal)); } -void CmdExecutor::StartCmdExecutorThread() +void CmdExecutor::ClientEventLoop(std::unique_ptr handler) { - SeqPacketSocketServer cmdExecutorMainSocket(CONTROL_SOCKET_NAME, MAX_WRITE_LOG_TASK); - if (cmdExecutorMainSocket.Init() < 0) { - cout << "Failed to init control socket ! \n"; - } else { - if (chmod(CONTROL_SOCKET, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) < 0) { - cout << "chmod control socket failed !\n"; - } - cout << "Begin to cmd accept !\n"; - cmdExecutorMainSocket.AcceptConnection(CmdExecutorThreadFunc); + decltype(m_clients)::iterator clientInfoIt; + { + std::lock_guard lg(m_clientAccess); + clientInfoIt = std::find_if(m_clients.begin(), m_clients.end(), + [](const std::unique_ptr& ct) { + return ct->m_clientThread.get_id() == std::this_thread::get_id(); + }); } + assert(clientInfoIt != m_clients.end()); + + prctl(PR_SET_NAME, "hilogd.query"); + auto logQuerier = std::make_shared(std::move(handler), m_hilogBuffer); + logQuerier->LogQuerierThreadFunc(logQuerier); + + // TODO add stopping child thread if parent thread is finished! + // this will cause changes inside log querier and will be done later. + // if ((*clientInfoIt)->m_stopThread.load()) { + // // break; return; :( + // } + + std::lock_guard ul(m_finishedClientAccess); + m_finishedClients.push_back(std::this_thread::get_id()); } -HilogBuffer* CmdExecutor::getHilogBuffer() +void CmdExecutor::CleanFinishedClients() { - return hilogBuffer; + std::list threadsToJoin; + { + // select clients to clean up - pick threads that we have to be sure are ended + std::scoped_lock sl(m_finishedClientAccess, m_clientAccess); + for (auto threadId : m_finishedClients) { + auto clientInfoIt = std::find_if(m_clients.begin(), m_clients.end(), + [&threadId](const std::unique_ptr& ct) { + return ct->m_clientThread.get_id() == threadId; + }); + if (clientInfoIt != m_clients.end()) { + threadsToJoin.push_back(std::move((*clientInfoIt)->m_clientThread)); + m_clients.erase(clientInfoIt); + } + } + m_finishedClients.clear(); + } + for (auto& thread : threadsToJoin) { + if (thread.joinable()) { + thread.join(); + } + } } + } // namespace HiviewDFX } // namespace OHOS diff --git a/services/hilogd/include/cmd_executor.h b/services/hilogd/include/cmd_executor.h index 8a3573a..93418b1 100644 --- a/services/hilogd/include/cmd_executor.h +++ b/services/hilogd/include/cmd_executor.h @@ -17,21 +17,34 @@ #include #include +#include +#include #include "log_buffer.h" -#include "log_querier.h" -#include "log_persister.h" -#include "seq_packet_socket_server.h" +#include + namespace OHOS { namespace HiviewDFX { +struct ClientThread { + std::thread m_clientThread; + std::atomic m_stopThread; +}; + class CmdExecutor { public: CmdExecutor(HilogBuffer* buffer); - void StartCmdExecutorThread(); - static HilogBuffer* getHilogBuffer(); - ~CmdExecutor() = default; + ~CmdExecutor(); + void MainLoop(); private: - static HilogBuffer* hilogBuffer; + void OnAcceptedConnection(std::unique_ptr handler); + void ClientEventLoop(std::unique_ptr handler); + void CleanFinishedClients(); + + HilogBuffer* m_hilogBuffer = nullptr; + std::list> m_clients; + std::mutex m_clientAccess; + std::vector m_finishedClients; + std::mutex m_finishedClientAccess; }; } // namespace HiviewDFX } // namespace OHOS diff --git a/services/hilogd/main.cpp b/services/hilogd/main.cpp index e95b4dd..4c1e7cf 100644 --- a/services/hilogd/main.cpp +++ b/services/hilogd/main.cpp @@ -105,7 +105,7 @@ int HilogdEntry(int argc, char* argv[]) startupCheckThread.detach(); CmdExecutor cmdExecutor(&hilogBuffer); - cmdExecutor.StartCmdExecutorThread(); + cmdExecutor.MainLoop(); return 0; } -- Gitee From e7f6b9f1162f0e4e03c2c9ba7c6b343220a86da3 Mon Sep 17 00:00:00 2001 From: Karol Grydziuszko Date: Mon, 29 Nov 2021 16:39:07 +0100 Subject: [PATCH 2/3] Stylistic changes regarding to clang tidy. Signed-off-by: Karol Grydziuszko Change-Id: I64b98facdc3c555e5b3822f88506d6e340124186 --- .../native/include/seq_packet_socket_server.h | 2 +- frameworks/native/linux_utils.cpp | 9 ++-- services/hilogd/cmd_executor.cpp | 46 ++++++++----------- services/hilogd/include/cmd_executor.h | 2 +- 4 files changed, 25 insertions(+), 34 deletions(-) diff --git a/frameworks/native/include/seq_packet_socket_server.h b/frameworks/native/include/seq_packet_socket_server.h index e9d0ce2..57fb9a5 100644 --- a/frameworks/native/include/seq_packet_socket_server.h +++ b/frameworks/native/include/seq_packet_socket_server.h @@ -17,8 +17,8 @@ #define SEQ_PACKET_SOCKET_SERVER_H #include "socket_server.h" + #include -#include namespace OHOS { namespace HiviewDFX { diff --git a/frameworks/native/linux_utils.cpp b/frameworks/native/linux_utils.cpp index bad09bf..6a9a777 100644 --- a/frameworks/native/linux_utils.cpp +++ b/frameworks/native/linux_utils.cpp @@ -21,7 +21,7 @@ using namespace std::literals; namespace OHOS { namespace HiviewDFX { namespace Utils { -std::string_view ListenErrorToStr(int error) +std::string_view ListenErrorToStr(int error) { static const std::unordered_map ERRORS { // explanations taken from https://man7.org/linux/man-pages/man2/listen.2.html @@ -50,7 +50,8 @@ std::string_view AcceptErrorToStr(int error) {EBADF, "sockfd is not an open file descriptor."sv}, {ECONNABORTED, "A connection has been aborted."sv}, {EFAULT, "The addr argument is not in a writable part of the user address space."sv}, - {EINTR, "The system call was interrupted by a signal that was caught before a valid connection arrived."sv}, + {EINTR, "The system call was interrupted by a signal " + "that was caught before a valid connection arrived."sv}, {EINVAL, "Socket is not listening for connections, or addrlen is invalid (e.g., is negative)."sv}, {EMFILE, "The per-process limit on the number of open file descriptors has been reached."sv}, {ENFILE, "The system-wide limit on the total number of open files has been reached."sv}, @@ -75,7 +76,8 @@ std::string_view ChmodErrorToStr(int error) // explanations taken from https://man7.org/linux/man-pages/man2/chmod.2.html {EACCES, "Search permission is denied on a component of the path prefix."sv}, {EBADF, "(fchmod()) The file descriptor fd is not valid."sv}, - {EBADF, "(fchmodat()) pathname is relative but dirfd is neither AT_FDCWD nor a valid file descriptor."sv}, + {EBADF, "(fchmodat()) pathname is relative but dirfd is neither " + "AT_FDCWD nor a valid file descriptor."sv}, {EFAULT, "pathname points outside your accessible address space."sv}, {EINVAL, "(fchmodat()) Invalid flag specified in flags."sv}, {EIO, "An I/O error occurred."sv}, @@ -112,7 +114,6 @@ std::string_view PollErrorToStr(int error) auto it = ERRORS.find(error); return it != ERRORS.end() ? it->second : "Unknown error"sv; } - } // Utils } // HiviewDFX } // OHOS diff --git a/services/hilogd/cmd_executor.cpp b/services/hilogd/cmd_executor.cpp index 5a0ba4e..255a87d 100644 --- a/services/hilogd/cmd_executor.cpp +++ b/services/hilogd/cmd_executor.cpp @@ -14,19 +14,21 @@ */ #include "cmd_executor.h" #include "log_querier.h" -#include "seq_packet_socket_server.h" -#include "linux_utils.h" +#include +#include + +#include +#include #include #include -#include -#include -#include -#include -#include -#include #include +#include + #include +#include +#include +#include namespace OHOS { namespace HiviewDFX { @@ -72,22 +74,19 @@ void CmdExecutor::MainLoop() std::cout << "Server started to listen !\n"; using namespace std::chrono_literals; - for(;;) { + for (;;) { const auto maxtime = 3000ms; short outEvent = 0; auto pollResult = cmdServer.Poll(POLLIN, outEvent, maxtime); - if (pollResult == 0) { - // std::cout << "---------Timeout---------\n"; + if (pollResult == 0) { // poll == 0 means timeout CleanFinishedClients(); continue; - } - else if (pollResult < 0) { + } else if (pollResult < 0) { int pollError = errno; std::cerr << "Socket polling error: " << pollError << "\n"; std::cerr << Utils::ChmodErrorToStr(pollError) << "\n"; break; - } - else if (pollResult != 1 || outEvent != POLLIN) { + } else if (pollResult != 1 || outEvent != POLLIN) { std::cerr << "Wrong poll result data." " Result: " << pollResult << " OutEvent: " << outEvent << "\n"; @@ -96,13 +95,11 @@ void CmdExecutor::MainLoop() int acceptResult = cmdServer.Accept(); if (acceptResult > 0) { - // std::cout << "---------New Connection---------\n"; int acceptedSockedFd = acceptResult; std::unique_ptr handler = std::make_unique(SOCK_SEQPACKET); handler->setHandler(acceptedSockedFd); OnAcceptedConnection(std::move(handler)); - } - else { + } else { int acceptError = errno; std::cerr << "Socket accept failed: " << acceptError << "\n"; std::cerr << Utils::AcceptErrorToStr(acceptError) << "\n"; @@ -114,10 +111,9 @@ void CmdExecutor::MainLoop() void CmdExecutor::OnAcceptedConnection(std::unique_ptr handler) { std::lock_guard lg(m_clientAccess); - auto newVal = std::unique_ptr(new ClientThread{ - std::thread(&CmdExecutor::ClientEventLoop, this, std::move(handler)), - std::atomic(false) - }); + auto newVal = std::make_unique(); + newVal->m_stopThread.store(false); + newVal->m_clientThread = std::thread(&CmdExecutor::ClientEventLoop, this, std::move(handler)); m_clients.push_back(std::move(newVal)); } @@ -137,12 +133,6 @@ void CmdExecutor::ClientEventLoop(std::unique_ptr handler) auto logQuerier = std::make_shared(std::move(handler), m_hilogBuffer); logQuerier->LogQuerierThreadFunc(logQuerier); - // TODO add stopping child thread if parent thread is finished! - // this will cause changes inside log querier and will be done later. - // if ((*clientInfoIt)->m_stopThread.load()) { - // // break; return; :( - // } - std::lock_guard ul(m_finishedClientAccess); m_finishedClients.push_back(std::this_thread::get_id()); } diff --git a/services/hilogd/include/cmd_executor.h b/services/hilogd/include/cmd_executor.h index 93418b1..fc67ee3 100644 --- a/services/hilogd/include/cmd_executor.h +++ b/services/hilogd/include/cmd_executor.h @@ -20,8 +20,8 @@ #include #include -#include "log_buffer.h" #include +#include "log_buffer.h" namespace OHOS { namespace HiviewDFX { -- Gitee From 4794d0d2f6ffbacb6d430ff2ab243022f095ab13 Mon Sep 17 00:00:00 2001 From: Karol Grydziuszko Date: Wed, 1 Dec 2021 12:12:09 +0100 Subject: [PATCH 3/3] Replaced own error to string implementation with strerror function. Better constant name. Signed-off-by: Karol Grydziuszko Change-Id: I15192a41140f625c51c572c700abb3e1a6bbf29a --- frameworks/native/BUILD.gn | 1 - frameworks/native/include/linux_utils.h | 32 ----- frameworks/native/linux_utils.cpp | 119 ------------------ .../native/seq_packet_socket_server.cpp | 8 +- services/hilogd/cmd_executor.cpp | 19 ++- 5 files changed, 11 insertions(+), 168 deletions(-) delete mode 100644 frameworks/native/include/linux_utils.h delete mode 100644 frameworks/native/linux_utils.cpp diff --git a/frameworks/native/BUILD.gn b/frameworks/native/BUILD.gn index a5f41f8..28a55a7 100644 --- a/frameworks/native/BUILD.gn +++ b/frameworks/native/BUILD.gn @@ -66,7 +66,6 @@ ohos_shared_library("libhilogutil") { "dgram_socket_client.cpp", "dgram_socket_server.cpp", "format.cpp", - "linux_utils.cpp", "seq_packet_socket_client.cpp", "seq_packet_socket_server.cpp", "socket.cpp", diff --git a/frameworks/native/include/linux_utils.h b/frameworks/native/include/linux_utils.h deleted file mode 100644 index 446fa33..0000000 --- a/frameworks/native/include/linux_utils.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright (c) 2021 Huawei Device Co., Ltd. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef HIVIEWDFX_LINUX_UTILS_H -#define HIVIEWDFX_LINUX_UTILS_H - -#include - -namespace OHOS { -namespace HiviewDFX { -namespace Utils { -std::string_view ListenErrorToStr(int error); -std::string_view AcceptErrorToStr(int error); -std::string_view ChmodErrorToStr(int error); -std::string_view PollErrorToStr(int error); -} // Utils -} // HiviewDFX -} // OHOS - -#endif // HIVIEWDFX_LINUX_UTILS_H diff --git a/frameworks/native/linux_utils.cpp b/frameworks/native/linux_utils.cpp deleted file mode 100644 index 6a9a777..0000000 --- a/frameworks/native/linux_utils.cpp +++ /dev/null @@ -1,119 +0,0 @@ -/* - * Copyright (c) 2021 Huawei Device Co., Ltd. - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "linux_utils.h" -#include -#include -using namespace std::literals; - -namespace OHOS { -namespace HiviewDFX { -namespace Utils { -std::string_view ListenErrorToStr(int error) -{ - static const std::unordered_map ERRORS { - // explanations taken from https://man7.org/linux/man-pages/man2/listen.2.html - {EADDRINUSE, "Another socket is already listening on the same port or " - "The socket referred to by sockfd " - "had not previously been bound to an address and, upon " - "attempting to bind it to an ephemeral port, it was " - "determined that all port numbers in the ephemeral port " - "range are currently in use."sv}, - {EBADF, "The argument sockfd is not a valid file descriptor."sv}, - {ENOTSOCK, "The file descriptor sockfd does not refer to a socket."sv}, - {EOPNOTSUPP, "The socket is not of a type that supports the listen() operation."sv}, - {0, "Success"sv} - }; - auto it = ERRORS.find(error); - return it != ERRORS.end() ? it->second : "Unknown error"sv; -} - -std::string_view AcceptErrorToStr(int error) -{ - static const std::unordered_map ERRORS { - // explanations taken from https://man7.org/linux/man-pages/man2/accept.2.html - {EOPNOTSUPP, "The socket is not of a type that supports the listen() operation."sv}, - {EAGAIN, "The socket is marked nonblocking and no connections are present to be accepted."sv}, - {EWOULDBLOCK, "The socket is marked nonblocking and no connections are present to be accepted."sv}, - {EBADF, "sockfd is not an open file descriptor."sv}, - {ECONNABORTED, "A connection has been aborted."sv}, - {EFAULT, "The addr argument is not in a writable part of the user address space."sv}, - {EINTR, "The system call was interrupted by a signal " - "that was caught before a valid connection arrived."sv}, - {EINVAL, "Socket is not listening for connections, or addrlen is invalid (e.g., is negative)."sv}, - {EMFILE, "The per-process limit on the number of open file descriptors has been reached."sv}, - {ENFILE, "The system-wide limit on the total number of open files has been reached."sv}, - {ENOBUFS, "Not enough free buffers. This often means that the memorsv " - "allocation is limited by the socket buffer limits, not by " - "the system memory."sv}, - {ENOMEM, "Not enough free memory. This often means that the memory " - "allocation is limited by the socket buffer limits, not by " - "the system memory."sv}, - {ENOTSOCK, "The file descriptor sockfd does not refer to a socket."sv}, - {EOPNOTSUPP, "The referenced socket is not of type SOCK_STREAM."sv}, - {EPERM, "Firewall rules forbid connection."sv}, - {EPROTO, "Protocol error."sv} - }; - auto it = ERRORS.find(error); - return it != ERRORS.end() ? it->second : "Unknown error"sv; -} - -std::string_view ChmodErrorToStr(int error) -{ - static const std::unordered_map ERRORS { - // explanations taken from https://man7.org/linux/man-pages/man2/chmod.2.html - {EACCES, "Search permission is denied on a component of the path prefix."sv}, - {EBADF, "(fchmod()) The file descriptor fd is not valid."sv}, - {EBADF, "(fchmodat()) pathname is relative but dirfd is neither " - "AT_FDCWD nor a valid file descriptor."sv}, - {EFAULT, "pathname points outside your accessible address space."sv}, - {EINVAL, "(fchmodat()) Invalid flag specified in flags."sv}, - {EIO, "An I/O error occurred."sv}, - {ELOOP, "Too many symbolic links were encountered in resolving pathname."sv}, - {ENAMETOOLONG, "pathname is too long."sv}, - {ENOENT, "The file does not exist."sv}, - {ENOMEM, "Insufficient kernel memory was available."sv}, - {ENOTDIR, "A component of the path prefix is not a directory."sv}, - {ENOTDIR, "(fchmodat()) pathname is relative and dirfd is a file " - "descriptor referring to a file other than a directory."sv}, - {ENOTSUP, "(fchmodat()) flags specified AT_SYMLINK_NOFOLLOW, which is not supported."sv}, - {EPERM, "The effective UID does not match the owner of the file, " - "and the process is not privileged (Linux: it does not have " - "the CAP_FOWNER capability)."sv}, - {EPERM, "The file is marked immutable or append-only. (See ioctl_iflags(2).)"sv}, - {EROFS, "The named file resides on a read-only filesystem."sv} - }; - auto it = ERRORS.find(error); - return it != ERRORS.end() ? it->second : "Unknown error"sv; -} - -std::string_view PollErrorToStr(int error) -{ - static const std::unordered_map ERRORS { - // explanations taken from https://man7.org/linux/man-pages/man2/poll.2.html - {EFAULT, "fds points outside the process's accessible address space. " - "The array given as argument was not contained in the " - "calling program's address space."sv}, - {EINTR, "A signal occurred before any requested event;"sv}, - {EINVAL, "The nfds value exceeds the RLIMIT_NOFILE value."sv}, - {EINVAL, "(ppoll()) The timeout value expressed in *ip is invalid (negative)."sv}, - {ENOMEM, "Unable to allocate memory for kernel data structures."sv}, - }; - auto it = ERRORS.find(error); - return it != ERRORS.end() ? it->second : "Unknown error"sv; -} -} // Utils -} // HiviewDFX -} // OHOS diff --git a/frameworks/native/seq_packet_socket_server.cpp b/frameworks/native/seq_packet_socket_server.cpp index 77f1d03..8b2ae34 100644 --- a/frameworks/native/seq_packet_socket_server.cpp +++ b/frameworks/native/seq_packet_socket_server.cpp @@ -15,9 +15,9 @@ #include "seq_packet_socket_server.h" -#include +#include #include -#include "linux_utils.h" +#include namespace OHOS { namespace HiviewDFX { @@ -27,7 +27,7 @@ int SeqPacketSocketServer::StartAcceptingConnection(AcceptingHandler onAccepted) if (listeningStatus < 0) { #ifdef DEBUG std::cerr << "Socket listen failed: " << listeningStatus << "\n"; - std::cerr << Utils::ListenErrorToStr(listeningStatus) << "\n"; + std::cerr << strerror(listeningStatus) << "\n"; #endif return listeningStatus; } @@ -46,7 +46,7 @@ int SeqPacketSocketServer::AcceptingLoop(AcceptingHandler func) int acceptError = errno; #ifdef DEBUG std::cerr << "Socket accept failed: " << acceptError << "\n"; - std::cerr << Utils::AccessErrorToStr(acceptError) << "\n"; + std::cerr < #include #include #include +#include #include #include #include @@ -32,7 +32,7 @@ namespace OHOS { namespace HiviewDFX { -const int MAX_WRITE_LOG_TASK = 100; +static const int MAX_CLIENT_CONNECTIONS = 100; CmdExecutor::CmdExecutor(HilogBuffer* buffer) { @@ -54,21 +54,16 @@ CmdExecutor::~CmdExecutor() void CmdExecutor::MainLoop() { - SeqPacketSocketServer cmdServer(CONTROL_SOCKET_NAME, MAX_WRITE_LOG_TASK); + SeqPacketSocketServer cmdServer(CONTROL_SOCKET_NAME, MAX_CLIENT_CONNECTIONS); if (cmdServer.Init() < 0) { std::cerr << "Failed to init control socket ! \n"; return; } - if (chmod(CONTROL_SOCKET, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH) < 0) { - int chmodError = errno; - std::cerr << "chmod control socket failed ! Error: " << chmodError << "\n"; - std::cerr << Utils::ChmodErrorToStr(chmodError) << "\n"; - } std::cout << "Begin to cmd accept !\n"; - int listeningStatus = cmdServer.Listen(MAX_WRITE_LOG_TASK); + int listeningStatus = cmdServer.Listen(MAX_CLIENT_CONNECTIONS); if (listeningStatus < 0) { std::cerr << "Socket listen failed: " << listeningStatus << "\n"; - std::cerr << Utils::ListenErrorToStr(listeningStatus) << "\n"; + std::cerr << strerror(listeningStatus) << "\n"; return; } std::cout << "Server started to listen !\n"; @@ -84,7 +79,7 @@ void CmdExecutor::MainLoop() } else if (pollResult < 0) { int pollError = errno; std::cerr << "Socket polling error: " << pollError << "\n"; - std::cerr << Utils::ChmodErrorToStr(pollError) << "\n"; + std::cerr << strerror(pollError) << "\n"; break; } else if (pollResult != 1 || outEvent != POLLIN) { std::cerr << "Wrong poll result data." @@ -102,7 +97,7 @@ void CmdExecutor::MainLoop() } else { int acceptError = errno; std::cerr << "Socket accept failed: " << acceptError << "\n"; - std::cerr << Utils::AcceptErrorToStr(acceptError) << "\n"; + std::cerr << strerror(acceptError) << "\n"; break; } } -- Gitee