This is an automated email from the ASF dual-hosted git repository.
luckychen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-weex.git
The following commit(s) were added to refs/heads/master by this push:
new af7c957 Feature/try fix top native crash fd problem (#2854)
af7c957 is described below
commit af7c957b21b476793cb904dd2e05245f5d31c31b
Author: darin <[email protected]>
AuthorDate: Wed Aug 28 16:33:13 2019 +0800
Feature/try fix top native crash fd problem (#2854)
* 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.
---
.../bridge/multi_process_and_so_initializer.cpp | 10 +-
.../bridge/multi_process_and_so_initializer.h | 2 +-
.../bridge/script_bridge_in_multi_process.cpp | 9 +-
.../bridge/script_bridge_in_multi_process.h | 4 +-
.../android/multiprocess/weex_js_connection.cpp | 105 ++++++++++++---------
.../android/multiprocess/weex_js_connection.h | 43 ++++++++-
6 files changed, 112 insertions(+), 61 deletions(-)
diff --git
a/weex_core/Source/android/bridge/multi_process_and_so_initializer.cpp
b/weex_core/Source/android/bridge/multi_process_and_so_initializer.cpp
index a16e31c..40ab7b3 100644
--- a/weex_core/Source/android/bridge/multi_process_and_so_initializer.cpp
+++ b/weex_core/Source/android/bridge/multi_process_and_so_initializer.cpp
@@ -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) {
@@ -52,7 +52,7 @@ startInitFrameWork:
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());
diff --git a/weex_core/Source/android/bridge/multi_process_and_so_initializer.h
b/weex_core/Source/android/bridge/multi_process_and_so_initializer.h
index 424e4d7..e972204 100644
--- a/weex_core/Source/android/bridge/multi_process_and_so_initializer.h
+++ b/weex_core/Source/android/bridge/multi_process_and_so_initializer.h
@@ -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
diff --git a/weex_core/Source/android/bridge/script_bridge_in_multi_process.cpp
b/weex_core/Source/android/bridge/script_bridge_in_multi_process.cpp
index 96ca9f0..7cc4ec6 100644
--- a/weex_core/Source/android/bridge/script_bridge_in_multi_process.cpp
+++ b/weex_core/Source/android/bridge/script_bridge_in_multi_process.cpp
@@ -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,
diff --git a/weex_core/Source/android/bridge/script_bridge_in_multi_process.h
b/weex_core/Source/android/bridge/script_bridge_in_multi_process.h
index 7296342..3d85c3f 100644
--- a/weex_core/Source/android/bridge/script_bridge_in_multi_process.h
+++ b/weex_core/Source/android/bridge/script_bridge_in_multi_process.h
@@ -24,6 +24,7 @@
class IPCHandler;
class WeexJSConnection;
+class WeexConnInfo;
class IPCHandler;
namespace WeexCore {
@@ -31,13 +32,10 @@ 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
diff --git a/weex_core/Source/android/multiprocess/weex_js_connection.cpp
b/weex_core/Source/android/multiprocess/weex_js_connection.cpp
index 11bd292..e4aefa8 100644
--- a/weex_core/Source/android/multiprocess/weex_js_connection.cpp
+++ b/weex_core/Source/android/multiprocess/weex_js_connection.cpp
@@ -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);
@@ -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!");
@@ -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();
@@ -151,10 +145,6 @@ WeexJSConnection::~WeexJSConnection() {
end();
}
-struct ThreadData {
- int ipcServerFd;
- IPCHandler *ipcServerHandler;
-};
// -1 unFinish, 0 error, 1 success
enum NewThreadStatus {
UNFINISH,
@@ -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();
@@ -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;
}
@@ -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();
@@ -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;
+}
diff --git a/weex_core/Source/android/multiprocess/weex_js_connection.h
b/weex_core/Source/android/multiprocess/weex_js_connection.h
index 50e4adf..23a032f 100644
--- a/weex_core/Source/android/multiprocess/weex_js_connection.h
+++ b/weex_core/Source/android/multiprocess/weex_js_connection.h
@@ -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;
