From 15a05b3639c47014642cf962bc8a4da1c991b30b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 7 Nov 2014 20:19:26 +0800 Subject: [PATCH 01/16] Add script to call symstore --- script/upload-windows-pdb.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 script/upload-windows-pdb.py diff --git a/script/upload-windows-pdb.py b/script/upload-windows-pdb.py new file mode 100644 index 000000000000..11b0b0967208 --- /dev/null +++ b/script/upload-windows-pdb.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python + +import os + +from lib.util import execute, rm_rf, safe_mkdir + + +SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) +SYMBOLS_DIR = 'dist\\symbols' +PDB_LIST = [ + 'out\\Release\\atom.exe.pdb', + 'vendor\\brightray\\vendor\\download\\libchromiumcontent\\Release\\chromiumcontent.dll.pdb', +] + + +def main(): + os.chdir(SOURCE_ROOT) + + rm_rf(SYMBOLS_DIR) + safe_mkdir(SYMBOLS_DIR) + for pdb in PDB_LIST: + run_symstore(pdb, SYMBOLS_DIR, 'AtomShell') + + +def run_symstore(pdb, dest, product): + execute(['symstore', 'add', '/r', '/f', pdb, '/s', dest, '/t', product]) + + +if __name__ == '__main__': + import sys + sys.exit(main()) From caa0634df88949985bd1c18ec99d8a07960c4407 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 7 Nov 2014 20:51:25 +0800 Subject: [PATCH 02/16] Upload symbols to S3 --- script/upload-windows-pdb.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/script/upload-windows-pdb.py b/script/upload-windows-pdb.py index 11b0b0967208..d7da8c13d049 100644 --- a/script/upload-windows-pdb.py +++ b/script/upload-windows-pdb.py @@ -1,8 +1,9 @@ #!/usr/bin/env python import os +import glob -from lib.util import execute, rm_rf, safe_mkdir +from lib.util import execute, rm_rf, safe_mkdir, s3put, s3_config SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) @@ -21,11 +22,19 @@ def main(): for pdb in PDB_LIST: run_symstore(pdb, SYMBOLS_DIR, 'AtomShell') + bucket, access_key, secret_key = s3_config() + files = glob.glob(SYMBOLS_DIR + '/*.pdb/*/*.pdb') + upload_symbols(bucket, access_key, secret_key, files) + def run_symstore(pdb, dest, product): execute(['symstore', 'add', '/r', '/f', pdb, '/s', dest, '/t', product]) +def upload_symbols(bucket, access_key, secret_key, files): + s3put(bucket, access_key, secret_key, SYMBOLS_DIR, 'atom-shell/symbols', files) + + if __name__ == '__main__': import sys sys.exit(main()) From 45fb3ec41d5ae36595183ab0baa17811eabb289d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 7 Nov 2014 21:45:40 +0800 Subject: [PATCH 03/16] Don't overwrite files on S3 --- script/lib/util.py | 1 + script/upload.py | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/script/lib/util.py b/script/lib/util.py index 83c3e05148b4..355fec027c45 100644 --- a/script/lib/util.py +++ b/script/lib/util.py @@ -183,6 +183,7 @@ def s3put(bucket, access_key, secret_key, prefix, key_prefix, files): '--secret_key', secret_key, '--prefix', prefix, '--key_prefix', key_prefix, + '--no_overwrite', '--grant', 'public-read' ] + files diff --git a/script/upload.py b/script/upload.py index 2ad7f5ffd8ef..edfa4c963c97 100755 --- a/script/upload.py +++ b/script/upload.py @@ -59,19 +59,19 @@ def main(): upload_atom_shell(github, release_id, os.path.join(DIST_DIR, CHROMEDRIVER_NAME)) - # Upload node's headers to S3. - bucket, access_key, secret_key = s3_config() - upload_node(bucket, access_key, secret_key, ATOM_SHELL_VERSION) - if args.publish_release: - # Press the publish button. - publish_release(github, release_id) + # Upload node's headers to S3. + bucket, access_key, secret_key = s3_config() + upload_node(bucket, access_key, secret_key, ATOM_SHELL_VERSION) # Upload the SHASUMS.txt. execute([sys.executable, os.path.join(SOURCE_ROOT, 'script', 'upload-checksums.py'), '-v', ATOM_SHELL_VERSION]) + # Press the publish button. + publish_release(github, release_id) + def parse_args(): parser = argparse.ArgumentParser(description='upload distribution file') From 34521e588055dbcface81ec86191aa9ef9d91cf5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 7 Nov 2014 22:52:00 +0800 Subject: [PATCH 04/16] Upload PDBs to Windows symbol server when publishing --- script/upload.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/script/upload.py b/script/upload.py index edfa4c963c97..7841017ed11e 100755 --- a/script/upload.py +++ b/script/upload.py @@ -69,6 +69,11 @@ def main(): os.path.join(SOURCE_ROOT, 'script', 'upload-checksums.py'), '-v', ATOM_SHELL_VERSION]) + # Upload PDBs to Windows symbol server. + if TARGET_PLATFORM == 'win32': + execute([sys.executable, + os.path.join(SOURCE_ROOT, 'script', 'upload-windows-pdb.py')]) + # Press the publish button. publish_release(github, release_id) From 621867e518ecba147774483d05b7f9f3591e971e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Nov 2014 16:09:13 +0800 Subject: [PATCH 05/16] docs: Setting up symbol server in debugger --- docs/README.md | 1 + docs/development/setting-up-symbol-server.md | 56 ++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 docs/development/setting-up-symbol-server.md diff --git a/docs/README.md b/docs/README.md index d22ef38e7238..ed7d803b72da 100644 --- a/docs/README.md +++ b/docs/README.md @@ -56,3 +56,4 @@ Modules for both sides: * [Build instructions (Mac)](development/build-instructions-mac.md) * [Build instructions (Windows)](development/build-instructions-windows.md) * [Build instructions (Linux)](development/build-instructions-linux.md) +* [Setting up symbol server in debugger](development/setting-up-symbol-server.md) diff --git a/docs/development/setting-up-symbol-server.md b/docs/development/setting-up-symbol-server.md new file mode 100644 index 000000000000..3b8b83dc90f0 --- /dev/null +++ b/docs/development/setting-up-symbol-server.md @@ -0,0 +1,56 @@ +# Setting up symbol server in debugger + +Debug symbols allow you to have better debugging sessions. They have information +about the functions contained in executables and dynamic libraries and provide +you with information to get clean call stacks. A Symbol Server allows the +debugger to load the correct symbols, binaries and sources automatically without +forcing users to download large debugging files. The server functions like +[Microsoft's symbol server](http://support.microsoft.com/kb/311503) so the +documentation there can be useful. + +Note that because released atom-shell builds are heavily optimized, debugging is +not always easy. The debugger will not be able to show you the content of all +variables and the execution path can seem strange because of inlining, tail +calls, and other compiler optimizations. The only workaround is to build an +unoptimized local build. + +The official symbol server URL for atom-shell is +http://symbols.mozilla.org/firefox. +You cannot visit this URL directly: you must add it to the symbol path of your +debugging tool. In the examples below, a local cache directory is used to avoid +repeatedly fetching the PDB from the server. Replace `c:\code\symbols` with an +appropriate cache directory on your machine. + +## Using the symbol server in Windbg + +The Windbg symbol path is configured with a string value delimited with asterisk +characters. To use only the atom-shell symbol server, add the following entry to +your symbol path (__note:__ you can replace `c:\code\symbols` with any writable +directory on your computer, if you'd prefer a different location for downloaded +symbols): + +``` +SRV*c:\code\symbols\*http://symbols.mozilla.org/firefox +``` + +Set this string as `_NT_SYMBOL_PATH` in the environment, using the Windbg menus, +or by typing the `.sympath` command. If you would like to get symbols from +Microsoft's symbol server as well, you should list that first: + +``` +SRV*c:\code\symbols\*http://msdl.microsoft.com/download/symbols;SRV*c:\code\symbols\*http://symbols.mozilla.org/firefox +``` + +## Using the symbol server in Visual Studio + + + + +## Troubleshooting: Symbols will not load + +Type the following commands in Windbg to print why symbols are not loading: + +``` +> !sym noisy +> .reload /f chromiumcontent.dll +``` From e5e94ed437d6a2a63d748f26374b2b192f44156d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Nov 2014 23:02:54 +0800 Subject: [PATCH 06/16] Add execution bit --- script/upload-windows-pdb.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 script/upload-windows-pdb.py diff --git a/script/upload-windows-pdb.py b/script/upload-windows-pdb.py old mode 100644 new mode 100755 From 6d663d1d0172b716e0dccc1f617b5a09b2905b67 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Nov 2014 23:07:57 +0800 Subject: [PATCH 07/16] Use lowercase for symbol paths --- script/upload-windows-pdb.py | 1 + 1 file changed, 1 insertion(+) diff --git a/script/upload-windows-pdb.py b/script/upload-windows-pdb.py index d7da8c13d049..c3e6863e50d2 100755 --- a/script/upload-windows-pdb.py +++ b/script/upload-windows-pdb.py @@ -24,6 +24,7 @@ def main(): bucket, access_key, secret_key = s3_config() files = glob.glob(SYMBOLS_DIR + '/*.pdb/*/*.pdb') + files = [f.lower() for f in files] upload_symbols(bucket, access_key, secret_key, files) From 4a1b6d76b2d4ebbe0b0b7a9dcac43d43e6eca73a Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Nov 2014 23:24:13 +0800 Subject: [PATCH 08/16] docs: Update symbol server address --- docs/development/setting-up-symbol-server.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/development/setting-up-symbol-server.md b/docs/development/setting-up-symbol-server.md index 3b8b83dc90f0..b69c0df02827 100644 --- a/docs/development/setting-up-symbol-server.md +++ b/docs/development/setting-up-symbol-server.md @@ -15,7 +15,7 @@ calls, and other compiler optimizations. The only workaround is to build an unoptimized local build. The official symbol server URL for atom-shell is -http://symbols.mozilla.org/firefox. +http://54.249.141.255:8086/atom-shell/symbols. You cannot visit this URL directly: you must add it to the symbol path of your debugging tool. In the examples below, a local cache directory is used to avoid repeatedly fetching the PDB from the server. Replace `c:\code\symbols` with an @@ -30,7 +30,7 @@ directory on your computer, if you'd prefer a different location for downloaded symbols): ``` -SRV*c:\code\symbols\*http://symbols.mozilla.org/firefox +SRV*c:\code\symbols\*http://54.249.141.255:8086/atom-shell/symbols ``` Set this string as `_NT_SYMBOL_PATH` in the environment, using the Windbg menus, @@ -38,7 +38,7 @@ or by typing the `.sympath` command. If you would like to get symbols from Microsoft's symbol server as well, you should list that first: ``` -SRV*c:\code\symbols\*http://msdl.microsoft.com/download/symbols;SRV*c:\code\symbols\*http://symbols.mozilla.org/firefox +SRV*c:\code\symbols\*http://msdl.microsoft.com/download/symbols;SRV*c:\code\symbols\*http://54.249.141.255:8086/atom-shell/symbols ``` ## Using the symbol server in Visual Studio From 90480fff4e2ad72efec8f2c97f01b3793b8dde42 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 10 Nov 2014 23:33:56 +0800 Subject: [PATCH 09/16] Fix pylint warnings --- script/upload-windows-pdb.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/script/upload-windows-pdb.py b/script/upload-windows-pdb.py index c3e6863e50d2..e03e2bcfac7c 100755 --- a/script/upload-windows-pdb.py +++ b/script/upload-windows-pdb.py @@ -8,9 +8,10 @@ from lib.util import execute, rm_rf, safe_mkdir, s3put, s3_config SOURCE_ROOT = os.path.abspath(os.path.dirname(os.path.dirname(__file__))) SYMBOLS_DIR = 'dist\\symbols' +DOWNLOAD_DIR = 'vendor\\brightray\\vendor\\download\\libchromiumcontent' PDB_LIST = [ 'out\\Release\\atom.exe.pdb', - 'vendor\\brightray\\vendor\\download\\libchromiumcontent\\Release\\chromiumcontent.dll.pdb', + DOWNLOAD_DIR + '\\Release\\chromiumcontent.dll.pdb', ] @@ -33,7 +34,8 @@ def run_symstore(pdb, dest, product): def upload_symbols(bucket, access_key, secret_key, files): - s3put(bucket, access_key, secret_key, SYMBOLS_DIR, 'atom-shell/symbols', files) + s3put(bucket, access_key, secret_key, SYMBOLS_DIR, 'atom-shell/symbols', + files) if __name__ == '__main__': From 212cd67f08be818c601ea1575a3824fa846f1c4f Mon Sep 17 00:00:00 2001 From: Paul Betts Date: Mon, 10 Nov 2014 11:53:12 -0800 Subject: [PATCH 10/16] Ports are limited to 64k --- docs/api/chrome-command-line-switches.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/chrome-command-line-switches.md b/docs/api/chrome-command-line-switches.md index 158e147d4626..6ac57ac97d88 100644 --- a/docs/api/chrome-command-line-switches.md +++ b/docs/api/chrome-command-line-switches.md @@ -7,7 +7,7 @@ module is emitted: ```javascript var app = require('app'); -app.commandLine.appendSwitch('remote-debugging-port', '88315'); +app.commandLine.appendSwitch('remote-debugging-port', '8315'); app.commandLine.appendSwitch('host-rules', 'MAP * 127.0.0.1'); app.on('ready', function() { From 739c432c98bfede81c85637f732c427d8e0798c2 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 12:24:33 +0800 Subject: [PATCH 11/16] linux: Writes crash report upload log --- .../linux/crash_dump_handler.cc | 134 ++++++++++++++---- .../crash_reporter/linux/crash_dump_handler.h | 1 + 2 files changed, 106 insertions(+), 29 deletions(-) diff --git a/atom/common/crash_reporter/linux/crash_dump_handler.cc b/atom/common/crash_reporter/linux/crash_dump_handler.cc index c5ae6ac94fae..2e7de5bf51ea 100644 --- a/atom/common/crash_reporter/linux/crash_dump_handler.cc +++ b/atom/common/crash_reporter/linux/crash_dump_handler.cc @@ -75,6 +75,10 @@ uint64_t kernel_timeval_to_ms(struct kernel_timeval *tv) { return ret; } +bool my_isxdigit(char c) { + return (c >= '0' && c <= '9') || ((c | 0x20) >= 'a' && (c | 0x20) <= 'f'); +} + size_t LengthWithoutTrailingSpaces(const char* str, size_t len) { while (len > 0 && str[len - 1] == ' ') { len--; @@ -345,13 +349,12 @@ void ExecUploadProcessOrTerminate(const BreakpadInfo& info, kWgetBinary, header, post_file, - // TODO(zcbenz): Enabling custom upload url. info.upload_url, "--timeout=60", // Set a timeout so we don't hang forever. "--tries=1", // Don't retry if the upload fails. "--quiet", // Be silent. "-O", // output reply to /dev/null. - "/dev/null", + "/dev/fd/3", NULL, }; static const char msg[] = "Cannot upload crash dump: cannot exec " @@ -361,6 +364,95 @@ void ExecUploadProcessOrTerminate(const BreakpadInfo& info, sys__exit(1); } +// Runs in the helper process to wait for the upload process running +// ExecUploadProcessOrTerminate() to finish. Returns the number of bytes written +// to |fd| and save the written contents to |buf|. +// |buf| needs to be big enough to hold |bytes_to_read| + 1 characters. +size_t WaitForCrashReportUploadProcess(int fd, size_t bytes_to_read, + char* buf) { + size_t bytes_read = 0; + + // Upload should finish in about 10 seconds. Add a few more 500 ms + // internals to account for process startup time. + for (size_t wait_count = 0; wait_count < 24; ++wait_count) { + struct kernel_pollfd poll_fd; + poll_fd.fd = fd; + poll_fd.events = POLLIN | POLLPRI | POLLERR; + int ret = sys_poll(&poll_fd, 1, 500); + if (ret < 0) { + // Error + break; + } else if (ret > 0) { + // There is data to read. + ssize_t len = HANDLE_EINTR( + sys_read(fd, buf + bytes_read, bytes_to_read - bytes_read)); + if (len < 0) + break; + bytes_read += len; + if (bytes_read == bytes_to_read) + break; + } + // |ret| == 0 -> timed out, continue waiting. + // or |bytes_read| < |bytes_to_read| still, keep reading. + } + buf[bytes_to_read] = 0; // Always NUL terminate the buffer. + return bytes_read; +} + +// |buf| should be |expected_len| + 1 characters in size and NULL terminated. +bool IsValidCrashReportId(const char* buf, size_t bytes_read, + size_t expected_len) { + if (bytes_read != expected_len) + return false; + for (size_t i = 0; i < bytes_read; ++i) { + if (!my_isxdigit(buf[i]) && buf[i] != '-') + return false; + } + return true; +} + +// |buf| should be |expected_len| + 1 characters in size and NULL terminated. +void HandleCrashReportId(const char* buf, size_t bytes_read, + size_t expected_len) { + WriteNewline(); + if (!IsValidCrashReportId(buf, bytes_read, expected_len)) { + static const char msg[] = "Failed to get crash dump id."; + WriteLog(msg, sizeof(msg) - 1); + WriteNewline(); + + static const char id_msg[] = "Report Id: "; + WriteLog(id_msg, sizeof(id_msg) - 1); + WriteLog(buf, bytes_read); + WriteNewline(); + return; + } + + // Write crash dump id to stderr. + static const char msg[] = "Crash dump id: "; + WriteLog(msg, sizeof(msg) - 1); + WriteLog(buf, my_strlen(buf)); + WriteNewline(); + + // Write crash dump id to crash log as: seconds_since_epoch,crash_id + struct kernel_timeval tv; + if (!sys_gettimeofday(&tv, NULL)) { + uint64_t time = kernel_timeval_to_ms(&tv) / 1000; + char time_str[kUint64StringSize]; + const unsigned time_len = my_uint64_len(time); + my_uint64tos(time_str, time, time_len); + + const int kLogOpenFlags = O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC; + int log_fd = sys_open("/tmp/uploads.log", kLogOpenFlags, 0600); + if (log_fd > 0) { + sys_write(log_fd, time_str, time_len); + sys_write(log_fd, ",", 1); + sys_write(log_fd, buf, my_strlen(buf)); + sys_write(log_fd, "\n", 1); + IGNORE_RET(sys_close(log_fd)); + } + } +} + } // namespace void HandleCrashDump(const BreakpadInfo& info) { @@ -616,33 +708,13 @@ void HandleCrashDump(const BreakpadInfo& info) { // Helper process. if (upload_child > 0) { IGNORE_RET(sys_close(fds[1])); - char id_buf[17]; // Crash report IDs are expected to be 16 chars. - ssize_t len = -1; - // Upload should finish in about 10 seconds. Add a few more 500 ms - // internals to account for process startup time. - for (size_t wait_count = 0; wait_count < 24; ++wait_count) { - struct kernel_pollfd poll_fd; - poll_fd.fd = fds[0]; - poll_fd.events = POLLIN | POLLPRI | POLLERR; - int ret = sys_poll(&poll_fd, 1, 500); - if (ret < 0) { - // Error - break; - } else if (ret > 0) { - // There is data to read. - len = HANDLE_EINTR(sys_read(fds[0], id_buf, sizeof(id_buf) - 1)); - break; - } - // ret == 0 -> timed out, continue waiting. - } - if (len > 0) { - // Write crash dump id to stderr. - id_buf[len] = 0; - static const char msg[] = "\nCrash dump id: "; - WriteLog(msg, sizeof(msg) - 1); - WriteLog(id_buf, my_strlen(id_buf)); - WriteLog("\n", 1); - } + + const size_t kCrashIdLength = 36; + char id_buf[kCrashIdLength + 1]; + size_t bytes_read = + WaitForCrashReportUploadProcess(fds[0], kCrashIdLength, id_buf); + HandleCrashReportId(id_buf, bytes_read, kCrashIdLength); + if (sys_waitpid(upload_child, NULL, WNOHANG) == 0) { // Upload process is still around, kill it. sys_kill(upload_child, SIGKILL); @@ -666,4 +738,8 @@ size_t WriteLog(const char* buf, size_t nbytes) { return sys_write(2, buf, nbytes); } +size_t WriteNewline() { + return WriteLog("\n", 1); +} + } // namespace crash_reporter diff --git a/atom/common/crash_reporter/linux/crash_dump_handler.h b/atom/common/crash_reporter/linux/crash_dump_handler.h index 9331a8fbd935..e2bb0b6a327f 100644 --- a/atom/common/crash_reporter/linux/crash_dump_handler.h +++ b/atom/common/crash_reporter/linux/crash_dump_handler.h @@ -32,6 +32,7 @@ struct BreakpadInfo { void HandleCrashDump(const BreakpadInfo& info); size_t WriteLog(const char* buf, size_t nbytes); +size_t WriteNewline(); } // namespace crash_reporter From f13d8407ee78407eb57a3dcccccc42687c830635 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 12:51:16 +0800 Subject: [PATCH 12/16] linux: Put crash dumps under "/tmp/ProductName Crashes/" --- atom/common/crash_reporter/crash_reporter_linux.cc | 12 ++++++------ atom/common/crash_reporter/crash_reporter_linux.h | 2 +- .../crash_reporter/linux/crash_dump_handler.cc | 1 - 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index ab12869ba740..dd7b6b41ba9a 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -12,9 +12,9 @@ #include "base/debug/crash_logging.h" #include "base/files/file_path.h" +#include "base/files/file_util.h" #include "base/linux_util.h" #include "base/logging.h" -#include "base/path_service.h" #include "base/process/memory.h" #include "base/memory/singleton.h" #include "vendor/breakpad/src/client/linux/handler/exception_handler.h" @@ -61,7 +61,7 @@ void CrashReporterLinux::InitBreakpad(const std::string& product_name, const std::string& submit_url, bool auto_submit, bool skip_system_crash_handler) { - EnableCrashDumping(); + EnableCrashDumping(product_name); crash_keys_.SetKeyValue("prod", "Atom-Shell"); crash_keys_.SetKeyValue("ver", version.c_str()); @@ -76,11 +76,11 @@ void CrashReporterLinux::SetUploadParameters() { upload_parameters_["platform"] = "linux"; } -void CrashReporterLinux::EnableCrashDumping() { - base::FilePath tmp_path("/tmp"); - PathService::Get(base::DIR_TEMP, &tmp_path); +void CrashReporterLinux::EnableCrashDumping(const std::string& product_name) { + std::string dump_dir = "/tmp/" + product_name + " Crashes"; + base::FilePath dumps_path(dump_dir); + base::CreateDirectory(dumps_path); - base::FilePath dumps_path(tmp_path); MinidumpDescriptor minidump_descriptor(dumps_path.value()); minidump_descriptor.set_size_limit(kMaxMinidumpFileSize); diff --git a/atom/common/crash_reporter/crash_reporter_linux.h b/atom/common/crash_reporter/crash_reporter_linux.h index 660c471d2a02..c1055aded110 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.h +++ b/atom/common/crash_reporter/crash_reporter_linux.h @@ -39,7 +39,7 @@ class CrashReporterLinux : public CrashReporter { CrashReporterLinux(); virtual ~CrashReporterLinux(); - void EnableCrashDumping(); + void EnableCrashDumping(const std::string& product_name); static bool CrashDone(const google_breakpad::MinidumpDescriptor& minidump, void* context, diff --git a/atom/common/crash_reporter/linux/crash_dump_handler.cc b/atom/common/crash_reporter/linux/crash_dump_handler.cc index 2e7de5bf51ea..4403d1d803fa 100644 --- a/atom/common/crash_reporter/linux/crash_dump_handler.cc +++ b/atom/common/crash_reporter/linux/crash_dump_handler.cc @@ -414,7 +414,6 @@ bool IsValidCrashReportId(const char* buf, size_t bytes_read, // |buf| should be |expected_len| + 1 characters in size and NULL terminated. void HandleCrashReportId(const char* buf, size_t bytes_read, size_t expected_len) { - WriteNewline(); if (!IsValidCrashReportId(buf, bytes_read, expected_len)) { static const char msg[] = "Failed to get crash dump id."; WriteLog(msg, sizeof(msg) - 1); From 02bcdc1c19b87a5f2cb365c39c68726ff6d1b4ae Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 13:30:06 +0800 Subject: [PATCH 13/16] linux: Put "uploads.log" under "/tmp/ProductName Crashes/" --- atom/common/crash_reporter/crash_reporter_linux.cc | 5 +++++ atom/common/crash_reporter/linux/crash_dump_handler.cc | 4 +++- atom/common/crash_reporter/linux/crash_dump_handler.h | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/atom/common/crash_reporter/crash_reporter_linux.cc b/atom/common/crash_reporter/crash_reporter_linux.cc index dd7b6b41ba9a..89e2f7e76a29 100644 --- a/atom/common/crash_reporter/crash_reporter_linux.cc +++ b/atom/common/crash_reporter/crash_reporter_linux.cc @@ -17,6 +17,7 @@ #include "base/logging.h" #include "base/process/memory.h" #include "base/memory/singleton.h" +#include "base/strings/stringprintf.h" #include "vendor/breakpad/src/client/linux/handler/exception_handler.h" #include "vendor/breakpad/src/common/linux/linux_libc_support.h" @@ -81,6 +82,10 @@ void CrashReporterLinux::EnableCrashDumping(const std::string& product_name) { base::FilePath dumps_path(dump_dir); base::CreateDirectory(dumps_path); + std::string log_file = base::StringPrintf( + "%s/%s", dump_dir.c_str(), "uploads.log"); + strncpy(g_crash_log_path, log_file.c_str(), sizeof(g_crash_log_path)); + MinidumpDescriptor minidump_descriptor(dumps_path.value()); minidump_descriptor.set_size_limit(kMaxMinidumpFileSize); diff --git a/atom/common/crash_reporter/linux/crash_dump_handler.cc b/atom/common/crash_reporter/linux/crash_dump_handler.cc index 4403d1d803fa..56a5e094d446 100644 --- a/atom/common/crash_reporter/linux/crash_dump_handler.cc +++ b/atom/common/crash_reporter/linux/crash_dump_handler.cc @@ -441,7 +441,7 @@ void HandleCrashReportId(const char* buf, size_t bytes_read, my_uint64tos(time_str, time, time_len); const int kLogOpenFlags = O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC; - int log_fd = sys_open("/tmp/uploads.log", kLogOpenFlags, 0600); + int log_fd = sys_open(g_crash_log_path, kLogOpenFlags, 0600); if (log_fd > 0) { sys_write(log_fd, time_str, time_len); sys_write(log_fd, ",", 1); @@ -454,6 +454,8 @@ void HandleCrashReportId(const char* buf, size_t bytes_read, } // namespace +char g_crash_log_path[256]; + void HandleCrashDump(const BreakpadInfo& info) { int dumpfd; bool keep_fd = false; diff --git a/atom/common/crash_reporter/linux/crash_dump_handler.h b/atom/common/crash_reporter/linux/crash_dump_handler.h index e2bb0b6a327f..f600c9e0d1b4 100644 --- a/atom/common/crash_reporter/linux/crash_dump_handler.h +++ b/atom/common/crash_reporter/linux/crash_dump_handler.h @@ -34,6 +34,9 @@ void HandleCrashDump(const BreakpadInfo& info); size_t WriteLog(const char* buf, size_t nbytes); size_t WriteNewline(); +// Global variable storing the path of upload log. +extern char g_crash_log_path[256]; + } // namespace crash_reporter #endif // ATOM_COMMON_CRASH_REPORTER_LINUX_CRASH_DUMP_HANDLER_H_ From 9a825c5cbd46f3fc963616a1191ff55bf84eb36f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 16:38:16 +0800 Subject: [PATCH 14/16] win: Writes uploads.log --- .../crash_reporter/win/crash_service.cc | 27 +++++++++++++++++++ spec/api-crash-reporter-spec.coffee | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/atom/common/crash_reporter/win/crash_service.cc b/atom/common/crash_reporter/win/crash_service.cc index 5cb6f9835138..ee77eb07f668 100644 --- a/atom/common/crash_reporter/win/crash_service.cc +++ b/atom/common/crash_reporter/win/crash_service.cc @@ -13,6 +13,8 @@ #include "base/command_line.h" #include "base/file_util.h" #include "base/logging.h" +#include "base/strings/string_number_conversions.h" +#include "base/time/time.h" #include "base/win/windows_version.h" #include "vendor/breakpad/src/client/windows/crash_generation/client_info.h" #include "vendor/breakpad/src/client/windows/crash_generation/crash_generation_server.h" @@ -66,6 +68,30 @@ bool WriteCustomInfoToFile(const std::wstring& dump_path, const CrashMap& map) { return true; } +bool WriteReportIDToFile(const std::wstring& dump_path, + const std::wstring& report_id) { + std::wstring file_path(dump_path); + size_t last_slash = file_path.rfind(L'\\'); + if (last_slash == std::wstring::npos) + return false; + file_path.resize(last_slash); + file_path += L"\\uploads.log"; + + std::wofstream file(file_path.c_str(), + std::ios_base::out | std::ios_base::app | std::ios::binary); + if (!file.is_open()) + return false; + + int64 seconds_since_epoch = + (base::Time::Now() - base::Time::UnixEpoch()).InSeconds(); + std::wstring line = base::Int64ToString16(seconds_since_epoch); + line += L','; + line += report_id; + line += L'\n'; + file.write(line.c_str(), static_cast(line.length())); + return true; +} + // The window procedure task is to handle when a) the user logs off. // b) the system shuts down or c) when the user closes the window. LRESULT __stdcall CrashSvcWndProc(HWND hwnd, UINT message, @@ -422,6 +448,7 @@ DWORD CrashService::AsyncSendDump(void* context) { ++info->self->requests_sent_; ++info->self->requests_handled_; retry_round = 0; + WriteReportIDToFile(info->dump_path, report_id); break; case google_breakpad::RESULT_THROTTLED: report_id = L""; diff --git a/spec/api-crash-reporter-spec.coffee b/spec/api-crash-reporter-spec.coffee index 2dd1499edb7d..43f7b2f03508 100644 --- a/spec/api-crash-reporter-spec.coffee +++ b/spec/api-crash-reporter-spec.coffee @@ -32,7 +32,7 @@ describe 'crash-reporter module', -> assert.equal fields['_version'], require('remote').require('app').getVersion() assert files['upload_file_minidump']['name']? - res.end() + res.end('abc-123-def') server.close() done() server.listen 0, '127.0.0.1', -> From 66e96f69fca7251678ffca89fcfb7b0f90b3977c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 17:20:04 +0800 Subject: [PATCH 15/16] Add crashRepoter.getLastCrashReport API --- atom/common/api/lib/crash-reporter.coffee | 33 ++++++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/atom/common/api/lib/crash-reporter.coffee b/atom/common/api/lib/crash-reporter.coffee index ecbd2f165ddc..6fd839de3f15 100644 --- a/atom/common/api/lib/crash-reporter.coffee +++ b/atom/common/api/lib/crash-reporter.coffee @@ -1,18 +1,21 @@ -{spawn} = require 'child_process' binding = process.atomBinding 'crash_reporter' +fs = require 'fs' +os = require 'os' +path = require 'path' +{spawn} = require 'child_process' class CrashReporter start: (options={}) -> - {productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra} = options + {@productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra} = options - productName ?= 'Atom-Shell' + @productName ?= 'Atom-Shell' companyName ?= 'GitHub, Inc' submitUrl ?= 'http://54.249.141.255:1127/post' autoSubmit ?= true ignoreSystemCrashHandler ?= false extra ?= {} - extra._productName ?= productName + extra._productName ?= @productName extra._companyName ?= companyName extra._version ?= if process.type is 'browser' @@ -20,12 +23,12 @@ class CrashReporter else require('remote').require('app').getVersion() - start = -> binding.start productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra + start = => binding.start @productName, companyName, submitUrl, autoSubmit, ignoreSystemCrashHandler, extra if process.platform is 'win32' args = [ "--reporter-url=#{submitUrl}" - "--application-name=#{productName}" + "--application-name=#{@productName}" "--v=1" ] env = ATOM_SHELL_INTERNAL_CRASH_SERVICE: 1 @@ -35,4 +38,20 @@ class CrashReporter else start() -module.exports = new CrashReporter + getLastCrashReport: -> + tmpdir = + if process.platform is 'win32' + os.tmpdir() + else + '/tmp' + log = path.join tmpdir, "#{@productName} Crashes", 'uploads.log' + try + reports = String(fs.readFileSync(log)).split('\n') + return null unless reports.length > 1 + [time, id] = reports[reports.length - 2].split ',' + return {date: new Date(parseInt(time) * 1000), id} + catch e + return null + +crashRepoter = new CrashReporter +module.exports = crashRepoter From 62fd76f7e415501bf587966110f4c793debc407e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 11 Nov 2014 20:20:36 +0800 Subject: [PATCH 16/16] docs: crashRepoter.getLastCrashReport --- docs/api/crash-reporter.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/api/crash-reporter.md b/docs/api/crash-reporter.md index 233dd1f1d619..3d0cf5467b82 100644 --- a/docs/api/crash-reporter.md +++ b/docs/api/crash-reporter.md @@ -27,6 +27,11 @@ crashReporter.start({ * Only string properties are send correctly. * Nested objects are not supported. +## crashReporter.getLastCrashReport() + +Returns the date and ID of last crash report, when there was no crash report +sent or the crash reporter is not started, `null` will be returned. + # crash-reporter payload The crash reporter will send the following data to the `submitUrl` as `POST`: