diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 50a507ea8..f92202935 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,7 +15,7 @@ on you machine: To get the source of `gprofiler`, clone the git repository via: ````bash -git clone https://github.com/granulate/gprofiler +git clone --recursive https://github.com/granulate/gprofiler ```` This will clone the complete source to your local machine. Navigate to the project folder diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index f2ba8ee84..cb5292294 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -25,7 +25,7 @@ java_exit_code_to_signo, locate_hotspot_error_file, ) -from granulate_utils.linux import proc_events +from granulate_utils.linux import proc_events, ns from granulate_utils.linux.ns import get_proc_root_path, resolve_proc_root_links, run_in_ns from granulate_utils.linux.oom import get_oom_entry from granulate_utils.linux.signals import get_signal_entry @@ -41,7 +41,6 @@ from gprofiler.profilers.registry import ProfilerArgument, register_profiler from gprofiler.utils import ( TEMPORARY_STORAGE_PATH, - get_process_nspid, is_process_running, pgrep_maps, process_comm, @@ -149,7 +148,7 @@ def __init__( self._process_root = get_proc_root_path(process) self._cmdline = process.cmdline() self._cwd = process.cwd() - self._nspid = get_process_nspid(self.process.pid) + self._nspid = ns.get_process_nspid(self.process.pid) # not using storage_dir for AP itself on purpose: this path should remain constant for the lifetime # of the target process, so AP is loaded exactly once (if we have multiple paths, AP can be loaded @@ -217,11 +216,6 @@ def _existing_realpath(self, path: str) -> Optional[str]: return path if os.path.exists(path) else None def locate_hotspot_error_file(self) -> Optional[str]: - # nspid is required - if self._nspid is None: - # TODO: fix get_process_nspid so it always succeeds - return None - for path in locate_hotspot_error_file(self._nspid, self._cmdline): realpath = self._existing_realpath(path) if realpath is not None: @@ -605,23 +599,20 @@ def _is_jvm_version_supported(self, java_version_cmd_output: str) -> bool: @staticmethod def _get_java_version(process: Process) -> str: - nspid = get_process_nspid(process.pid) - if nspid is not None: - # this has the benefit of working even if the Java binary was replaced, e.g due to an upgrade. - # in that case, the libraries would have been replaced as well, and therefore we're actually checking - # the version of the now installed Java, and not the running one. - # but since this is used for the "JDK type" check, it's good enough - we don't expect that to change. - # this whole check, however, is growing to be too complex, and we should consider other approaches - # for it: - # 1. purely in async-profiler - before calling any APIs that might harm blacklisted JDKs, we can - # check the JDK type in async-profiler itself. - # 2. assume JDK type by the path, e.g the "java" Docker image has - # "/usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java" which means "OpenJDK". needs to be checked for - # other JDK types. - java_path = f"/proc/{nspid}/exe" - else: - # TODO fix get_process_nspid() for all cases. - java_path = os.readlink(f"/proc/{process.pid}/exe") + nspid = ns.get_process_nspid(process.pid) + + # this has the benefit of working even if the Java binary was replaced, e.g due to an upgrade. + # in that case, the libraries would have been replaced as well, and therefore we're actually checking + # the version of the now installed Java, and not the running one. + # but since this is used for the "JDK type" check, it's good enough - we don't expect that to change. + # this whole check, however, is growing to be too complex, and we should consider other approaches + # for it: + # 1. purely in async-profiler - before calling any APIs that might harm blacklisted JDKs, we can + # check the JDK type in async-profiler itself. + # 2. assume JDK type by the path, e.g the "java" Docker image has + # "/usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java" which means "OpenJDK". needs to be checked for + # other JDK types. + java_path = f"/proc/{nspid}/exe" java_version_cmd_output = None diff --git a/gprofiler/utils.py b/gprofiler/utils.py index 84e831291..d36010a07 100644 --- a/gprofiler/utils.py +++ b/gprofiler/utils.py @@ -62,20 +62,6 @@ def is_root() -> bool: return os.geteuid() == 0 -def get_process_nspid(pid: int) -> Optional[int]: - with open(f"/proc/{pid}/status") as f: - for line in f: - fields = line.split() - if fields[0] == "NSpid:": - return int(fields[-1]) - - # old kernel (pre 4.1) with no NSpid. - # TODO if needed, this can be implemented for pre 4.1, by reading all /proc/pid/sched files as - # seen by the PID NS; they expose the init NS PID (due to a bug fixed in 4.14~), and we can get the NS PID - # from the listing of those files itself. - return None - - libc: Optional[ctypes.CDLL] = None diff --git a/granulate-utils b/granulate-utils index 26f3c7e7b..83dd35e62 160000 --- a/granulate-utils +++ b/granulate-utils @@ -1 +1 @@ -Subproject commit 26f3c7e7b2bcbf39cf6d32f74761667ff45be595 +Subproject commit 83dd35e626dd414762db6959b26f77fab18ad1e8