Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

Commit

Permalink
Feature/try fix top native crash fd problem (#2854)
Browse files Browse the repository at this point in the history
* add pid and genId to ashmem

* try fix fd problem

所有的 fd 操作都控制在局部变量. 去掉所有的静态变量. 防止全局污染

* Rename client name to file name and return base if fd create failed

* make sure close fd happened in deconstructor.
  • Loading branch information
Darin726 authored and lucky-chen committed Aug 28, 2019
1 parent 8722cf3 commit af7c957
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@
namespace WeexCore {

bool MultiProcessAndSoInitializer::Init(const std::function<void(IPCHandler*)>& OnHandlerCreated,
const std::function<bool(std::unique_ptr<WeexJSConnection>, std::unique_ptr<IPCHandler>, std::unique_ptr<IPCHandler>)>& OnInitFinished,
const std::function<bool(std::unique_ptr<WeexJSConnection>)>& OnInitFinished,
const std::function<void(const char*, const char*, const char*)>& ReportException){
bool reinit = false;
LOGE("MultiProcessAndSoInitializer IS IN init");
startInitFrameWork:
try {
auto handler = std::move(createIPCHandler());
auto server_handler = std::move(createIPCHandler());
OnHandlerCreated(server_handler.get());
std::unique_ptr<WeexJSConnection> connection(new WeexJSConnection());
auto sender = connection->start(handler.get(), server_handler.get(), reinit);
std::unique_ptr<WeexJSConnection> connection(new WeexJSConnection(new WeexConnInfo(std::move(createIPCHandler()), true),
new WeexConnInfo(std::move(server_handler), false)));
auto sender = connection->start(reinit);
if (sender == nullptr) {
LOGE("JSFramwork init start sender is null");
if (!reinit) {
Expand All @@ -52,7 +52,7 @@ bool MultiProcessAndSoInitializer::Init(const std::function<void(IPCHandler*)>&
return false;
}
} else {
OnInitFinished(std::move(connection), std::move(handler), std::move(server_handler));
OnInitFinished(std::move(connection));
}
} catch (IPCException& e) {
LOGE("WeexProxy catch:%s", e.msg());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class MultiProcessAndSoInitializer {
virtual ~MultiProcessAndSoInitializer() {}

bool Init(const std::function<void(IPCHandler*)>&,
const std::function<bool(std::unique_ptr<WeexJSConnection>, std::unique_ptr<IPCHandler>, std::unique_ptr<IPCHandler>)>&,
const std::function<bool(std::unique_ptr<WeexJSConnection>)>&,
const std::function<void(const char*, const char*, const char*)>&);
};
} // namespace WeexCore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1004,16 +1004,11 @@ ScriptBridgeInMultiProcess::ScriptBridgeInMultiProcess() {
LOGE("ScriptBridgeInMultiProcess");
bool passable = initializer->Init(
[this](IPCHandler *handler) { RegisterIPCCallback(handler); },
[this](std::unique_ptr<WeexJSConnection> connection,
std::unique_ptr<IPCHandler> handler,
std::unique_ptr<IPCHandler> server_handler) {
[this](std::unique_ptr<WeexJSConnection> connection) {
static_cast<bridge::script::ScriptSideInMultiProcess *>(script_side())
->set_sender(connection->sender());
connection_ = std::move(connection);
handler_ = std::move(handler);
server_handler_ = std::move(server_handler);
LOGE("ScriptBridgeInMultiProcess finish %p %p", server_handler_.get(),
server_handler.get());
LOGE("ScriptBridgeInMultiProcess finish");
return true;
},
[this](const char *page_id, const char *func,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,18 @@

class IPCHandler;
class WeexJSConnection;
class WeexConnInfo;
class IPCHandler;
namespace WeexCore {

class ScriptBridgeInMultiProcess : public ScriptBridge {
public:
ScriptBridgeInMultiProcess();
~ScriptBridgeInMultiProcess();

void RegisterIPCCallback(IPCHandler *handler);

private:
std::unique_ptr<WeexJSConnection> connection_;
std::unique_ptr<IPCHandler> handler_;
std::unique_ptr<IPCHandler> server_handler_;
DISALLOW_COPY_AND_ASSIGN(ScriptBridgeInMultiProcess);
};
} // namespace WeexCore
Expand Down
105 changes: 62 additions & 43 deletions weex_core/Source/android/multiprocess/weex_js_connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@
static bool s_in_find_icu = false;
static std::string g_crashFileName;

volatile static bool fd_server_closed = false;

static void doExec(int fdClient, int fdServer, bool traceEnable, bool startupPie);

static int copyFile(const char *SourceFile, const char *NewFile);
Expand All @@ -56,13 +54,6 @@ static void closeAllButThis(int fd, int fd2);

static void printLogOnFile(const char *log);

static void closeServerFd(int fd) {
if(fd_server_closed)
return;
close(fd);
fd_server_closed = true;
}

static bool checkOrCreateCrashFile(const char* file) {
if (file == nullptr) {
LOGE("checkOrCreateCrashFile Pass error file name!");
Expand Down Expand Up @@ -121,8 +112,11 @@ struct WeexJSConnection::WeexJSConnectionImpl {
pid_t child{0};
};

WeexJSConnection::WeexJSConnection()
WeexJSConnection::WeexJSConnection(WeexConnInfo* client, WeexConnInfo *server)
: m_impl(new WeexJSConnectionImpl) {
this->client_.reset(client);
this->server_.reset(server);

if (SoUtils::crash_file_path() != nullptr) {
if (checkDirOrFileIsLink(SoUtils::crash_file_path())) {
std::string tmp = SoUtils::crash_file_path();
Expand Down Expand Up @@ -151,10 +145,6 @@ WeexJSConnection::~WeexJSConnection() {
end();
}

struct ThreadData {
int ipcServerFd;
IPCHandler *ipcServerHandler;
};
// -1 unFinish, 0 error, 1 success
enum NewThreadStatus {
UNFINISH,
Expand All @@ -165,19 +155,18 @@ enum NewThreadStatus {
static volatile int newThreadStatus = UNFINISH;

static void *newIPCServer(void *_td) {
ThreadData *td = static_cast<ThreadData *>(_td);
void *base = mmap(nullptr, IPCFutexPageQueue::ipc_size, PROT_READ | PROT_WRITE, MAP_SHARED,
td->ipcServerFd, 0);
WeexConnInfo *server = static_cast<WeexConnInfo *>(_td);
void *base = server->mmap_for_ipc();

if (base == MAP_FAILED) {
LOGE("newIPCServer start map filed errno %d ", errno);
int _errno = errno;
close(td->ipcServerFd);
//throw IPCException("failed to map ashmem region: %s", strerror(_errno));
newThreadStatus = ERROR;
return nullptr;
}

IPCHandler *handler = td->ipcServerHandler;
IPCHandler *handler = server->handler.get();
std::unique_ptr<IPCFutexPageQueue> futexPageQueue(
new IPCFutexPageQueue(base, IPCFutexPageQueue::ipc_size, 0));
const std::unique_ptr<IPCHandler> &testHandler = createIPCHandler();
Expand All @@ -190,49 +179,42 @@ static void *newIPCServer(void *_td) {
listener->listen();
} catch (IPCException &e) {
LOGE("IPCException server died %s",e.msg());
closeServerFd(td->ipcServerFd);
base::android::DetachFromVM();
pthread_exit(NULL);
}
return nullptr;
}

IPCSender *WeexJSConnection::start(bool reinit) {
if(client_== nullptr || client_.get() == nullptr) {
return nullptr;
}

IPCSender *WeexJSConnection::start(IPCHandler *handler, IPCHandler *serverHandler, bool reinit) {
int fd = ashmem_create_region("WEEX_IPC_CLIENT", IPCFutexPageQueue::ipc_size);
if (-1 == fd) {
throw IPCException("failed to create ashmem region: %s", strerror(errno));
if(server_ == nullptr || server_.get() == nullptr) {
return nullptr;
}
void *base = mmap(nullptr, IPCFutexPageQueue::ipc_size, PROT_READ | PROT_WRITE, MAP_SHARED,
fd, 0);

void *base = client_->mmap_for_ipc();
if (base == MAP_FAILED) {
int _errno = errno;
close(fd);
throw IPCException("failed to map ashmem region: %s", strerror(_errno));
}

std::unique_ptr<IPCFutexPageQueue> futexPageQueue(
new IPCFutexPageQueue(base, IPCFutexPageQueue::ipc_size, 0));
std::unique_ptr<IPCSender> sender(createIPCSender(futexPageQueue.get(), handler));
std::unique_ptr<IPCSender> sender(createIPCSender(futexPageQueue.get(), client_->handler.get()));
m_impl->serverSender = std::move(sender);
m_impl->futexPageQueue = std::move(futexPageQueue);

int fd2 = ashmem_create_region("WEEX_IPC_SERVER", IPCFutexPageQueue::ipc_size);
if (-1 == fd2) {
throw IPCException("failed to create ashmem region: %s", strerror(errno));
}
fd_server_closed = false;
ThreadData td = { static_cast<int>(fd2), static_cast<IPCHandler *>(serverHandler) };

pthread_attr_t threadAttr;
newThreadStatus = UNFINISH;

pthread_attr_init(&threadAttr);
pthread_t ipcServerThread;
int i = pthread_create(&ipcServerThread, &threadAttr, newIPCServer, &td);
int i = pthread_create(&ipcServerThread, &threadAttr, newIPCServer, server_.get());
if(i != 0) {
throw IPCException("failed to create ipc server thread");
}

while (newThreadStatus == UNFINISH) {
continue;
}
Expand Down Expand Up @@ -282,23 +264,19 @@ IPCSender *WeexJSConnection::start(IPCHandler *handler, IPCHandler *serverHandle
if (child == -1) {
int myerrno = errno;
munmap(base, IPCFutexPageQueue::ipc_size);
close(fd);
closeServerFd(fd2);
throw IPCException("failed to fork: %s", strerror(myerrno));
} else if (child == 0) {
LOGE("weexcore fork child success\n");
// the child
closeAllButThis(fd, fd2);
closeAllButThis(client_->ipcFd, server_->ipcFd);
// implements close all but handles[1]
// do exec
doExec(fd, fd2, true, startupPie);
doExec(client_->ipcFd, server_->ipcFd, true, startupPie);
LOGE("exec Failed completely.");
// failed to exec
_exit(1);
} else {
printLogOnFile("fork success on main process and start m_impl->futexPageQueue->spinWaitPeer()");
close(fd);
closeServerFd(fd2);
m_impl->child = child;
try {
m_impl->futexPageQueue->spinWaitPeer();
Expand Down Expand Up @@ -617,3 +595,44 @@ int copyFile(const char *SourceFile, const char *NewFile) {
return 1;
}
}

void *WeexConnInfo::mmap_for_ipc() {
pid_t pid = getpid();
std::string fileName(this->is_client ? "WEEX_IPC_CLIENT" : "WEEX_IPC_SERVER");
fileName += std::to_string(pid);
int fd = -1;
int initTimes = 1;
void *base = MAP_FAILED;
do {
fd = ashmem_create_region(fileName.c_str(), IPCFutexPageQueue::ipc_size);
if (-1 == fd) {
if (this->is_client) {
throw IPCException("failed to create ashmem region: %s", strerror(errno));
} else {
LOGE("failed to create ashmem region: %s", strerror(errno))
return base;
}
}
base = mmap(nullptr, IPCFutexPageQueue::ipc_size, PROT_READ | PROT_WRITE, MAP_SHARED,
fd, 0);
if (base == MAP_FAILED) {
close(fd);
fd = -1;
int _errno = errno;
initTimes++;
if (_errno == EBADF || _errno == EACCES) {
LOGE("start map filed errno %d should retry", errno);
continue;
} else {
if (this->is_client) {
throw IPCException("start map filed errno %d", errno);
} else {
LOGE("start map filed errno %d", errno)
}
break;
}
}
} while ((initTimes <= 2) && base == MAP_FAILED);
this->ipcFd = fd;
return base;
}
43 changes: 41 additions & 2 deletions weex_core/Source/android/multiprocess/weex_js_connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,61 @@
#define WEEXJSCONNECTION_H

#include <memory>
#include <unistd.h>
#include <sys/mman.h>

class IPCSender;
class IPCHandler;


class WeexConnInfo {
public:
std::unique_ptr<IPCHandler> handler;
int ipcFd;

WeexConnInfo(std::unique_ptr<IPCHandler> handler, bool isClient) {
this->handler = std::move(handler);
ipcFd = -1;
is_client = isClient;
}

~WeexConnInfo() {
closeFd();
}

void *mmap_for_ipc();

void closeFd() {
if(ipcFd == -1) {
return;
}

if(hasBeenClosed)
return;
hasBeenClosed = true;
close(ipcFd);
}

private:
bool hasBeenClosed = false;
bool is_client = false;
};

class WeexJSConnection {
public:
WeexJSConnection();
WeexJSConnection(WeexConnInfo* client, WeexConnInfo *server);

~WeexJSConnection();

IPCSender *start(IPCHandler *handler, IPCHandler *serverHandler, bool reinit);
IPCSender *start(bool reinit);

void end();

IPCSender* sender();

std::unique_ptr<WeexConnInfo> client_;
std::unique_ptr<WeexConnInfo> server_;

private:
struct WeexJSConnectionImpl;

Expand Down

0 comments on commit af7c957

Please sign in to comment.