LocIpc derefs nullptr if stopping blocking listener

LocIpcRunnable is created only for nonBlocking listeners.
But stopListening would use LocIpcRunnable not knowing if
this is a blocking or nonblocking listener. LocIpcRunnable
is no longer a shared resource between the two threads. We
use a copy of string to keep the needed socket name.

Change-Id: Ib5c57a89f630807d6d03f3fbc698a56b9ffddf4d
CRs-Fixed: 2431397
This commit is contained in:
Kevin Tang 2019-04-05 19:46:29 -07:00
parent ac8ce3c7d5
commit 07db01d9af
2 changed files with 24 additions and 20 deletions

View file

@ -33,6 +33,8 @@
#include <log_util.h> #include <log_util.h>
#include "LocIpc.h" #include "LocIpc.h"
using std::string;
namespace loc_util { namespace loc_util {
#ifdef LOG_TAG #ifdef LOG_TAG
@ -47,7 +49,7 @@ namespace loc_util {
class LocIpcRunnable : public LocRunnable { class LocIpcRunnable : public LocRunnable {
friend LocIpc; friend LocIpc;
public: public:
LocIpcRunnable(LocIpc& locIpc, const std::string& ipcName) LocIpcRunnable(LocIpc& locIpc, const string& ipcName)
: mLocIpc(locIpc), mIpcName(ipcName) {} : mLocIpc(locIpc), mIpcName(ipcName) {}
bool run() override { bool run() override {
if (!mLocIpc.startListeningBlocking(mIpcName)) { if (!mLocIpc.startListeningBlocking(mIpcName)) {
@ -58,17 +60,17 @@ public:
} }
private: private:
LocIpc& mLocIpc; LocIpc& mLocIpc;
const std::string mIpcName; const string mIpcName;
}; };
bool LocIpc::startListeningNonBlocking(const std::string& name) { bool LocIpc::startListeningNonBlocking(const string& name) {
mRunnable = new LocIpcRunnable(*this, name); auto runnable = new LocIpcRunnable(*this, name);
std::string threadName("LocIpc-"); string threadName("LocIpc-");
threadName.append(name); threadName.append(name);
return mThread.start(threadName.c_str(), mRunnable); return mThread.start(threadName.c_str(), runnable);
} }
bool LocIpc::startListeningBlocking(const std::string& name) { bool LocIpc::startListeningBlocking(const string& name) {
bool stopRequested = false; bool stopRequested = false;
int fd = socket(AF_UNIX, SOCK_DGRAM, 0); int fd = socket(AF_UNIX, SOCK_DGRAM, 0);
@ -90,13 +92,14 @@ bool LocIpc::startListeningBlocking(const std::string& name) {
LOC_LOGe("bind socket error. reason:%s", strerror(errno)); LOC_LOGe("bind socket error. reason:%s", strerror(errno));
} else { } else {
mIpcFd = fd; mIpcFd = fd;
mIpcName = name;
// inform that the socket is ready to receive message // inform that the socket is ready to receive message
onListenerReady(); onListenerReady();
ssize_t nBytes = 0; ssize_t nBytes = 0;
std::string msg = ""; string msg = "";
std::string abort = LOC_MSG_ABORT; string abort = LOC_MSG_ABORT;
while (1) { while (1) {
msg.resize(LOC_MSG_BUF_LEN); msg.resize(LOC_MSG_BUF_LEN);
nBytes = ::recvfrom(fd, (void*)(msg.data()), msg.size(), 0, NULL, NULL); nBytes = ::recvfrom(fd, (void*)(msg.data()), msg.size(), 0, NULL, NULL);
@ -147,20 +150,19 @@ bool LocIpc::startListeningBlocking(const std::string& name) {
} }
void LocIpc::stopListening() { void LocIpc::stopListening() {
const char *socketName = nullptr;
if (mIpcFd >= 0) { if (mIpcFd >= 0) {
std::string abort = LOC_MSG_ABORT; string abort = LOC_MSG_ABORT;
socketName = (reinterpret_cast<LocIpcRunnable *>(mRunnable))->mIpcName.c_str(); if (!mIpcName.empty()) {
send(socketName, abort); send(mIpcName.c_str(), abort);
}
mIpcFd = -1; mIpcFd = -1;
} }
if (mRunnable) { if (!mIpcName.empty()) {
mRunnable = nullptr; mIpcName.clear();
} }
} }
bool LocIpc::send(const char name[], const std::string& data) { bool LocIpc::send(const char name[], const string& data) {
return send(name, (const uint8_t*)data.c_str(), data.length()); return send(name, (const uint8_t*)data.c_str(), data.length());
} }
@ -195,7 +197,7 @@ bool LocIpc::sendData(int fd, const sockaddr_un &addr, const uint8_t data[], uin
result = false; result = false;
} }
} else { } else {
std::string head = LOC_MSG_HEAD; string head = LOC_MSG_HEAD;
head.append(std::to_string(length)); head.append(std::to_string(length));
if (::sendto(fd, head.c_str(), head.length(), 0, if (::sendto(fd, head.c_str(), head.length(), 0,
(struct sockaddr*)&addr, sizeof(addr)) < 0) { (struct sockaddr*)&addr, sizeof(addr)) < 0) {

View file

@ -37,6 +37,8 @@
#include <sys/un.h> #include <sys/un.h>
#include <LocThread.h> #include <LocThread.h>
using std::string;
namespace loc_util { namespace loc_util {
class LocIpcSender; class LocIpcSender;
@ -44,7 +46,7 @@ class LocIpcSender;
class LocIpc { class LocIpc {
friend LocIpcSender; friend LocIpcSender;
public: public:
inline LocIpc() : mIpcFd(-1), mRunnable(nullptr) {} inline LocIpc() : mIpcFd(-1) {}
inline virtual ~LocIpc() { stopListening(); } inline virtual ~LocIpc() { stopListening(); }
// Listen for new messages in current thread. Calling this funciton will // Listen for new messages in current thread. Calling this funciton will
@ -94,7 +96,7 @@ private:
int mIpcFd; int mIpcFd;
LocThread mThread; LocThread mThread;
LocRunnable *mRunnable; string mIpcName;
}; };
class LocIpcSender { class LocIpcSender {