Skip to content

Commit

Permalink
Fix breakpad build + add test canary (pytorch#60990)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: pytorch#60990

This makes the breakpad build more explicit in its messaging and hints to cmake where to look for the library (it wasn't able to find it without `PATHS` on CI even though that works locally). This also adds a smoke test that will fail if breakpad isn't present on a CI job where it is expected (e.g. binary builds).

Test Plan: Imported from OSS

Reviewed By: malfet

Differential Revision: D29514316

Pulled By: driazati

fbshipit-source-id: 79514363334788f311ba5d4f25deed3452f0c3eb
  • Loading branch information
driazati authored and facebook-github-bot committed Jul 6, 2021
1 parent b6024b9 commit 45cc207
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 26 deletions.
15 changes: 12 additions & 3 deletions caffe2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -995,15 +995,24 @@ endif()


if(LINUX)
find_library(BREAKPAD_LIB breakpad_client)
find_path(BREAKPAD_INCLUDE_DIR breakpad)
find_library(BREAKPAD_LIB breakpad_client
PATHS /usr/local/lib/)
find_path(BREAKPAD_INCLUDE_DIR breakpad
PATHS /usr/local/include/)

if(BREAKPAD_LIB AND BREAKPAD_INCLUDE_DIR)
message(STATUS "found breakpad library")
target_link_libraries(torch_cpu PRIVATE ${BREAKPAD_LIB})
target_compile_definitions(torch_cpu PRIVATE ADD_BREAKPAD_SIGNAL_HANDLER)
target_include_directories(torch_cpu PRIVATE ${BREAKPAD_INCLUDE_DIR}/breakpad)
else()
message(STATUS "breakpad library not found")
if(BREAKPAD_INCLUDE_DIR)
message(STATUS "breakpad_client library not found")
elseif(BREAKPAD_LIB)
message(STATUS "breakpad include path not found")
else()
message(STATUS "breakpad_client library and include path not found")
endif()
endif()
endif()

Expand Down
14 changes: 2 additions & 12 deletions test/test_cpp_extensions_jit.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import torch.backends.cudnn
import torch.utils.cpp_extension
from torch.utils.cpp_extension import CUDA_HOME, ROCM_HOME
from torch.testing._internal.common_utils import gradcheck, TEST_WITH_ASAN
from torch.testing._internal.common_utils import gradcheck, TEST_WITH_ASAN, has_breakpad


TEST_CUDA = torch.cuda.is_available() and CUDA_HOME is not None
Expand All @@ -30,16 +30,6 @@
IS_WINDOWS = sys.platform == "win32"


def check_breakpad():
try:
torch._C._get_minidump_directory()
return True
except RuntimeError as e:
return "Minidump handler is uninintialized, make sure to call" in str(e)

HAS_BREAKPAD = check_breakpad()


def remove_build_path():
if sys.platform == "win32":
print("Not wiping extensions build folder because Windows")
Expand Down Expand Up @@ -877,7 +867,7 @@ def test_custom_compound_op_autograd(self):

gradcheck(torch.ops.my.add, [a, b], eps=1e-2)

@unittest.skipIf(not HAS_BREAKPAD, "Breakpad library must be present on system for crash handler")
@unittest.skipIf(not has_breakpad(), "Breakpad library must be present on system for crash handler")
@unittest.skipIf(TEST_WITH_ASAN, "ASAN disables the crash handler's signal handler")
def test_crash_handler(self):
def run_test(stderr_file, destination):
Expand Down
13 changes: 2 additions & 11 deletions test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import torch.hub as hub
from torch.autograd._functions.utils import check_onnx_broadcast
from torch.onnx.symbolic_opset9 import _prepare_onnx_paddings
from torch.testing._internal.common_utils import load_tests, retry, IS_SANDCASTLE, IS_WINDOWS
from torch.testing._internal.common_utils import load_tests, retry, IS_SANDCASTLE, IS_WINDOWS, has_breakpad
from urllib.error import URLError

# load_tests from torch.testing._internal.common_utils is used to automatically filter tests for
Expand All @@ -28,15 +28,6 @@

HAS_CUDA = torch.cuda.is_available()

def check_breakpad():
try:
torch._C._get_minidump_directory() # type: ignore[attr-defined]
return True
except RuntimeError as e:
return "Minidump handler is uninintialized, make sure to call" in str(e)

HAS_BREAKPAD = check_breakpad()


from torch.testing._internal.common_utils import TestCase, run_tests

Expand Down Expand Up @@ -748,7 +739,7 @@ def forward(self, x):


class TestCrashHandler(TestCase):
@unittest.skipIf(not HAS_BREAKPAD, "Crash handler lib was not linked in")
@unittest.skipIf(not has_breakpad(), "Crash handler lib was not linked in")
def test_python_exception_writing(self):
with tempfile.TemporaryDirectory() as temp_dir:
torch.utils._crash_handler.enable_minidumps(temp_dir)
Expand Down
9 changes: 9 additions & 0 deletions torch/testing/_internal/common_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2509,6 +2509,7 @@ def wrapped(self, *args, **kwargs):
f(self, *args, **kwargs, coalesced=False)
return wrapped


@contextlib.contextmanager
def disable_gc():
if gc.isenabled():
Expand All @@ -2519,3 +2520,11 @@ def disable_gc():
gc.enable()
else:
yield

def has_breakpad() -> bool:
# If not on a special build, check that the library was actually linked in
try:
torch._C._get_minidump_directory() # type: ignore[attr-defined]
return True
except RuntimeError as e:
return False

0 comments on commit 45cc207

Please sign in to comment.