From 3297d9234e15515aa91cc887b3b12db7e1040b02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Thu, 17 Feb 2022 17:04:42 +0100 Subject: [PATCH] Switch to `ProcessHandle` for getting the PID (#14842) * RELNOTES: Refactor system suspend event handling. We are looking to add more build anomaly reporting, so refactor the current system suspend handling to match the new model where we have a module and an event that we can record. Note this deprecates the AnomalyRecord out of BES for the time being. PiperOrigin-RevId: 409502784 * Switch to `ProcessHandle` for getting the pid. Uses new Java API instead of a native method. PiperOrigin-RevId: 409973910 * fix test Co-authored-by: dmaclach Co-authored-by: jhorvitz --- .../java/com/google/devtools/build/lib/BUILD | 1 - .../google/devtools/build/lib/analysis/BUILD | 1 - .../build/lib/analysis/NoBuildEvent.java | 6 +- .../analysis/NoBuildRequestFinishedEvent.java | 2 +- .../com/google/devtools/build/lib/bazel/BUILD | 1 + .../devtools/build/lib/bazel/Bazel.java | 1 + .../BuildCompletingEvent.java | 19 +-- .../proto/build_event_stream.proto | 3 +- .../build/lib/buildtool/BuildResult.java | 12 -- .../build/lib/buildtool/BuildTool.java | 14 +-- .../buildevent/BuildCompleteEvent.java | 6 +- .../buildevent/BuildStartingEvent.java | 3 +- .../buildevent/SystemSuspensionEvent.java | 73 +++++++++++ .../buildevent/TestingCompleteEvent.java | 9 +- .../google/devtools/build/lib/platform/BUILD | 8 +- .../build/lib/platform/SuspendCounter.java | 37 ------ .../lib/platform/SystemSuspensionModule.java | 65 ++++++++++ .../devtools/build/lib/runtime/commands/BUILD | 2 - .../lib/runtime/commands/CleanCommand.java | 3 +- .../lib/runtime/commands/NoTestsFound.java | 4 +- .../lib/runtime/commands/TestCommand.java | 17 +-- .../commands/info/ServerPidInfoItem.java | 3 +- src/main/native/unix_jni.cc | 116 +++++++++++++++--- src/main/native/unix_jni.h | 22 +++- src/main/native/unix_jni_bsd.cc | 3 +- src/main/native/unix_jni_darwin.cc | 76 ++++++------ src/main/native/unix_jni_linux.cc | 3 +- src/main/native/windows/BUILD | 2 +- ...ni.cc => system_suspension_monitor_jni.cc} | 13 +- .../buildtool/util/BlazeRuntimeWrapper.java | 5 +- .../google/devtools/build/lib/platform/BUILD | 12 +- .../lib/platform/SuspendCounterTest.java | 49 -------- .../platform/SystemSuspensionEventTest.java | 76 ++++++++++++ .../google/devtools/build/remote/worker/BUILD | 1 - .../build/remote/worker/RemoteWorker.java | 5 +- 35 files changed, 423 insertions(+), 250 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/buildtool/buildevent/SystemSuspensionEvent.java delete mode 100644 src/main/java/com/google/devtools/build/lib/platform/SuspendCounter.java create mode 100644 src/main/java/com/google/devtools/build/lib/platform/SystemSuspensionModule.java rename src/main/native/windows/{suspend_counter_jni.cc => system_suspension_monitor_jni.cc} (73%) delete mode 100644 src/test/java/com/google/devtools/build/lib/platform/SuspendCounterTest.java create mode 100644 src/test/java/com/google/devtools/build/lib/platform/SystemSuspensionEventTest.java diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index ccf94ebeb343b5..4f744c2f91bf26 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -290,7 +290,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/packages/semantics", "//src/main/java/com/google/devtools/build/lib/pkgcache", - "//src/main/java/com/google/devtools/build/lib/platform:suspend_counter", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/profiler:google-auto-profiler-utils", "//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index ce55b174f16cd0..ab553b460b52a1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -874,7 +874,6 @@ java_library( ":blaze_version_info", "//src/main/java/com/google/devtools/build/lib/buildeventstream", "//src/main/java/com/google/devtools/build/lib/buildeventstream/proto:build_event_stream_java_proto", - "//src/main/java/com/google/devtools/build/lib/util:process", "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/NoBuildEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/NoBuildEvent.java index e124cb4649241c..952d4330855e58 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/NoBuildEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/NoBuildEvent.java @@ -22,8 +22,6 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId; import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; import com.google.devtools.build.lib.buildeventstream.ProgressEvent; -import com.google.devtools.build.lib.util.ProcessUtils; -import java.util.Collection; /** This event raised to indicate that no build will be happening for the given command. */ public final class NoBuildEvent implements BuildEvent { @@ -55,7 +53,7 @@ public NoBuildEvent() { } @Override - public Collection getChildrenEvents() { + public ImmutableList getChildrenEvents() { if (separateFinishedEvent) { return ImmutableList.of( ProgressEvent.INITIAL_PROGRESS_UPDATE, BuildEventIdUtil.buildFinished()); @@ -83,7 +81,7 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert if (id != null) { started.setUuid(id); } - started.setServerPid(ProcessUtils.getpid()); + started.setServerPid(ProcessHandle.current().pid()); return GenericBuildEvent.protoChaining(this).setStarted(started.build()).build(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/NoBuildRequestFinishedEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/NoBuildRequestFinishedEvent.java index 5d45b81032a2b9..f1294f2c47fe5b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/NoBuildRequestFinishedEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/NoBuildRequestFinishedEvent.java @@ -20,6 +20,6 @@ /** {@link BuildEvent} indicating that a request that does not involve building as finished. */ public final class NoBuildRequestFinishedEvent extends BuildCompletingEvent { public NoBuildRequestFinishedEvent(ExitCode exitCode, long finishTimeMillis) { - super(exitCode, finishTimeMillis, false); + super(exitCode, finishTimeMillis); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index 8aee6bfeaa9c70..968c540f26c1dd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -145,6 +145,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/outputfilter", "//src/main/java/com/google/devtools/build/lib/packages/metrics", "//src/main/java/com/google/devtools/build/lib/platform:sleep_prevention_module", + "//src/main/java/com/google/devtools/build/lib/platform:system_suspension_module", "//src/main/java/com/google/devtools/build/lib/profiler/callcounts:callcounts_module", "//src/main/java/com/google/devtools/build/lib/profiler/memory:allocationtracker_module", "//src/main/java/com/google/devtools/build/lib/remote", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java index bb44b7d9108e4e..c6c4df23b2aae3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java @@ -44,6 +44,7 @@ public final class Bazel { com.google.devtools.build.lib.runtime.NoSpawnCacheModule.class, com.google.devtools.build.lib.runtime.CommandLogModule.class, com.google.devtools.build.lib.platform.SleepPreventionModule.class, + com.google.devtools.build.lib.platform.SystemSuspensionModule.class, com.google.devtools.build.lib.runtime.BazelFileSystemModule.class, com.google.devtools.build.lib.runtime.mobileinstall.MobileInstallModule.class, com.google.devtools.build.lib.bazel.BazelWorkspaceStatusModule.class, diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java index 39d948a8f156c3..8390012989bfc8 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/BuildCompletingEvent.java @@ -30,24 +30,17 @@ public abstract class BuildCompletingEvent implements BuildEvent { private final ExitCode exitCode; private final long finishTimeMillis; - /** Was the build suspended mid-build (e.g. hardware sleep, SIGSTOP). */ - private final boolean wasSuspended; - private final Collection children; public BuildCompletingEvent( - ExitCode exitCode, - long finishTimeMillis, - Collection children, - boolean wasSuspended) { + ExitCode exitCode, long finishTimeMillis, Collection children) { this.exitCode = exitCode; this.finishTimeMillis = finishTimeMillis; this.children = children; - this.wasSuspended = wasSuspended; } - public BuildCompletingEvent(ExitCode exitCode, long finishTimeMillis, boolean wasSuspended) { - this(exitCode, finishTimeMillis, ImmutableList.of(), wasSuspended); + public BuildCompletingEvent(ExitCode exitCode, long finishTimeMillis) { + this(exitCode, finishTimeMillis, ImmutableList.of()); } public ExitCode getExitCode() { @@ -72,18 +65,12 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert .setCode(exitCode.getNumericExitCode()) .build(); - BuildEventStreamProtos.BuildFinished.AnomalyReport protoAnamolyReport = - BuildEventStreamProtos.BuildFinished.AnomalyReport.newBuilder() - .setWasSuspended(wasSuspended) - .build(); - BuildEventStreamProtos.BuildFinished finished = BuildEventStreamProtos.BuildFinished.newBuilder() .setOverallSuccess(ExitCode.SUCCESS.equals(exitCode)) .setExitCode(protoExitCode) .setFinishTime(Timestamps.fromMillis(finishTimeMillis)) .setFinishTimeMillis(finishTimeMillis) - .setAnomalyReport(protoAnamolyReport) .build(); return GenericBuildEvent.protoChaining(this).setFinished(finished).build(); } diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index e9e00583cf5195..0fca2680156433 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto @@ -792,6 +792,7 @@ message BuildFinished { // Examples of suspensions are SIGSTOP, or the hardware being put to sleep. // If was_suspended is true, then most of the timings for this build are // suspect. + // NOTE: This is no longer set and is deprecated. bool was_suspended = 1; } @@ -812,7 +813,7 @@ message BuildFinished { // End of the build. google.protobuf.Timestamp finish_time = 5; - AnomalyReport anomaly_report = 4; + AnomalyReport anomaly_report = 4 [deprecated = true]; } message BuildMetrics { diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java index f11e2b5e37273f..d4133be7511567 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildResult.java @@ -48,8 +48,6 @@ public final class BuildResult { private long startTimeMillis = 0; // milliseconds since UNIX epoch. private long stopTimeMillis = 0; - private boolean wasSuspended = false; - private Throwable crash = null; private boolean catastrophe = false; private boolean stopOnFirstFailure; @@ -97,16 +95,6 @@ public double getElapsedSeconds() { return (stopTimeMillis - startTimeMillis) / 1000.0; } - /** Record if the build was suspended (SIGSTOP or hardware put to sleep). */ - public void setWasSuspended(boolean wasSuspended) { - this.wasSuspended = wasSuspended; - } - - /** Whether the build was suspended (SIGSTOP or hardware put to sleep). */ - public boolean getWasSuspended() { - return wasSuspended; - } - public void setDetailedExitCode(DetailedExitCode detailedExitCode) { this.detailedExitCode = detailedExitCode; } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java index d74071f8e2dc4a..528027b1efde3d 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java @@ -13,8 +13,6 @@ // limitations under the License. package com.google.devtools.build.lib.buildtool; -import static com.google.devtools.build.lib.platform.SuspendCounter.suspendCount; - import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; @@ -438,7 +436,6 @@ public BuildResult processRequest( BuildRequest request, TargetValidator validator, PostBuildCallback postBuildCallback) { BuildResult result = new BuildResult(request.getStartTime()); maybeSetStopOnFirstFailure(request, result); - int startSuspendCount = suspendCount(); Throwable crash = null; DetailedExitCode detailedExitCode = null; try { @@ -536,7 +533,7 @@ public BuildResult processRequest( new IllegalStateException("Unspecified DetailedExitCode")); } try (SilentCloseable c = Profiler.instance().profile("stopRequest")) { - stopRequest(result, crash, detailedExitCode, startSuspendCount); + stopRequest(result, crash, detailedExitCode); } } @@ -576,16 +573,10 @@ private static boolean needsExecutionPhase(BuildRequestOptions options) { * @param crash any unexpected {@link RuntimeException} or {@link Error}, may be null * @param detailedExitCode describes the exit code and an optional detailed failure value to add * to {@code result} - * @param startSuspendCount number of suspensions before the build started */ public void stopRequest( - BuildResult result, - @Nullable Throwable crash, - DetailedExitCode detailedExitCode, - int startSuspendCount) { + BuildResult result, @Nullable Throwable crash, DetailedExitCode detailedExitCode) { Preconditions.checkState((crash == null) || !detailedExitCode.isSuccess()); - int stopSuspendCount = suspendCount(); - Preconditions.checkState(startSuspendCount <= stopSuspendCount); result.setUnhandledThrowable(crash); result.setDetailedExitCode(detailedExitCode); @@ -598,7 +589,6 @@ public void stopRequest( } // The stop time has to be captured before we send the BuildCompleteEvent. result.setStopTime(runtime.getClock().currentTimeMillis()); - result.setWasSuspended(stopSuspendCount > startSuspendCount); // Skip the build complete events so that modules can run blazeShutdownOnCrash without thinking // that the build completed normally. BlazeCommandDispatcher will call handleCrash. diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildCompleteEvent.java index 04dc81b019702a..2b77fa5b0d9b4d 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildCompleteEvent.java @@ -32,11 +32,7 @@ public final class BuildCompleteEvent extends BuildCompletingEvent { /** Construct the BuildCompleteEvent. */ public BuildCompleteEvent(BuildResult result, Collection children) { - super( - result.getDetailedExitCode().getExitCode(), - result.getStopTime(), - children, - result.getWasSuspended()); + super(result.getDetailedExitCode().getExitCode(), result.getStopTime(), children); this.result = checkNotNull(result); } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java index 6cd8ff7d8cddab..f0ea90f17a750e 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/BuildStartingEvent.java @@ -26,7 +26,6 @@ import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.runtime.CommandLineEvent; -import com.google.devtools.build.lib.util.ProcessUtils; import com.google.protobuf.util.Timestamps; import java.util.Collection; @@ -107,7 +106,7 @@ public BuildEventStreamProtos.BuildEvent asStreamProto(BuildEventContext convert .setBuildToolVersion(BlazeVersionInfo.instance().getVersion()) .setOptionsDescription(request.getOptionsDescription()) .setCommand(request.getCommandName()) - .setServerPid(ProcessUtils.getpid()); + .setServerPid(ProcessHandle.current().pid()); if (pwd != null) { started.setWorkingDirectory(pwd); } diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/SystemSuspensionEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/SystemSuspensionEvent.java new file mode 100644 index 00000000000000..b7a1f5f470ea3a --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/SystemSuspensionEvent.java @@ -0,0 +1,73 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.buildtool.buildevent; + +import com.google.devtools.build.lib.events.ExtendedEventHandler; + +/** + * This event is fired from {@link + * com.google.devtools.build.lib.platform.SystemSuspensionModule#suspendCallback} to indicate that + * the user either suspended the build with a signal or put their computer to sleep. + */ +public class SystemSuspensionEvent implements ExtendedEventHandler.Postable { + + /** The possible reasons a system could be suspended. */ + public enum Reason { + SIGTSTP("Signal SIGTSTP"), + SIGCONT("Signal SIGCONT"), + SLEEP("Computer put to sleep"), + WAKE("Computer woken up"); + + private final String logString; + + Reason(String logString) { + this.logString = logString; + } + + public String logString() { + return logString; + } + + static Reason fromInt(int number) { + switch (number) { + case 0: + return SIGTSTP; + case 1: + return SIGCONT; + case 2: + return SLEEP; + case 3: + return WAKE; + default: + throw new IllegalStateException("Unknown suspension reason: " + number); + } + } + }; + + /** These constants are mapped to enum in third_party/bazel/src/main/native/unix_jni.h. */ + private final Reason reason; + + public SystemSuspensionEvent(int reason) { + this.reason = Reason.fromInt(reason); + } + + public Reason getReason() { + return reason; + } + + public String logString() { + return "SystemSuspensionEvent: " + reason.logString(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java index faa33782c44883..96780f90009f27 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/buildevent/TestingCompleteEvent.java @@ -32,13 +32,8 @@ public class TestingCompleteEvent extends BuildCompletingEvent { * * @param exitCode the overall exit code of "bazel test". * @param finishTimeMillis the finish time in milliseconds since the epoch. - * @param wasSuspended was the build suspended at any point. */ - public TestingCompleteEvent(ExitCode exitCode, long finishTimeMillis, boolean wasSuspended) { - super( - exitCode, - finishTimeMillis, - ImmutableList.of(BuildEventIdUtil.buildToolLogs()), - wasSuspended); + public TestingCompleteEvent(ExitCode exitCode, long finishTimeMillis) { + super(exitCode, finishTimeMillis, ImmutableList.of(BuildEventIdUtil.buildToolLogs())); } } diff --git a/src/main/java/com/google/devtools/build/lib/platform/BUILD b/src/main/java/com/google/devtools/build/lib/platform/BUILD index 0445f442ecaeee..503b556cad8155 100644 --- a/src/main/java/com/google/devtools/build/lib/platform/BUILD +++ b/src/main/java/com/google/devtools/build/lib/platform/BUILD @@ -21,12 +21,16 @@ java_library( ) java_library( - name = "suspend_counter", + name = "system_suspension_module", srcs = [ - "SuspendCounter.java", + "SystemSuspensionModule.java", ], deps = [ + "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/jni", + "//third_party:flogger", + "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/platform/SuspendCounter.java b/src/main/java/com/google/devtools/build/lib/platform/SuspendCounter.java deleted file mode 100644 index 8ef12a49bb0aaa..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/platform/SuspendCounter.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.platform; - -import com.google.devtools.build.lib.jni.JniLoader; - -/** Native methods for dealing with suspension events. */ -public final class SuspendCounter { - - static { - JniLoader.loadJni(); - } - - private SuspendCounter() {} - - static native int suspendCountJNI(); - - /** - * The number of times the build has been suspended. Currently this is a hardware sleep and/or the - * platform equivalents to a SIGSTOP/SIGTSTP. - */ - public static int suspendCount() { - return JniLoader.isJniAvailable() ? suspendCountJNI() : 0; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/platform/SystemSuspensionModule.java b/src/main/java/com/google/devtools/build/lib/platform/SystemSuspensionModule.java new file mode 100644 index 00000000000000..9a9b9db54d8009 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/platform/SystemSuspensionModule.java @@ -0,0 +1,65 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.platform; + +import com.google.common.flogger.GoogleLogger; +import com.google.devtools.build.lib.buildtool.buildevent.SystemSuspensionEvent; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.jni.JniLoader; +import com.google.devtools.build.lib.runtime.BlazeModule; +import com.google.devtools.build.lib.runtime.CommandEnvironment; +import javax.annotation.concurrent.GuardedBy; + +/** Detects suspension events. */ +public final class SystemSuspensionModule extends BlazeModule { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + + static { + JniLoader.loadJni(); + } + + @GuardedBy("this") + private Reporter reporter; + + private native void registerJNI(); + + public SystemSuspensionModule() { + if (JniLoader.isJniAvailable()) { + registerJNI(); + } + } + + @Override + public synchronized void beforeCommand(CommandEnvironment env) { + this.reporter = env.getReporter(); + } + + @Override + public synchronized void afterCommand() { + this.reporter = null; + } + + /** Callback method called from JNI whenever a suspension event occurs. */ + synchronized void suspendCallback(int reason) { + if (reporter != null) { + SystemSuspensionEvent event = new SystemSuspensionEvent(reason); + String logString = event.logString(); + reporter.handle(Event.info(logString)); + reporter.post(event); + logger.atInfo().log(logString); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD index 2a382fdd065d7b..626bf683f1305d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/BUILD @@ -84,7 +84,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:filetype", "//src/main/java/com/google/devtools/build/lib/util:interrupted_failure_details", "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/main/java/com/google/devtools/build/lib/util:process", "//src/main/java/com/google/devtools/build/lib/util:shell_escaper", "//src/main/java/com/google/devtools/build/lib/util:string", "//src/main/java/com/google/devtools/build/lib/util/io", @@ -94,7 +93,6 @@ java_library( "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/common/options", "//src/main/java/com/google/devtools/common/options:invocation_policy", - "//src/main/protobuf:analysis_java_proto", "//src/main/protobuf:bazel_flags_java_proto", "//src/main/protobuf:command_server_java_proto", "//src/main/protobuf:extra_actions_base_java_proto", diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java index 6295e7ba52aa5d..248dee29326e9b 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java @@ -40,7 +40,6 @@ import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; -import com.google.devtools.build.lib.util.ProcessUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; @@ -210,7 +209,7 @@ public static boolean canUseAsync(boolean async, boolean expunge, OS os, Reporte private static void asyncClean(CommandEnvironment env, Path path, String pathItemName) throws IOException, CommandException, InterruptedException { String tempBaseName = - path.getBaseName() + "_tmp_" + ProcessUtils.getpid() + "_" + UUID.randomUUID(); + path.getBaseName() + "_tmp_" + ProcessHandle.current().pid() + "_" + UUID.randomUUID(); // Keeping tempOutputBase in the same directory ensures it remains in the // same file system, and therefore the mv will be atomic and fast. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/NoTestsFound.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/NoTestsFound.java index 64b7c4854fa60d..8c2179032bc1ad 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/NoTestsFound.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/NoTestsFound.java @@ -20,7 +20,7 @@ /** This event is posted by the {@link TestCommand} if no tests were found. */ public class NoTestsFound extends BuildCompletingEvent { - public NoTestsFound(ExitCode exitCode, long finishTimeMillis, boolean wasSuspended) { - super(exitCode, finishTimeMillis, wasSuspended); + public NoTestsFound(ExitCode exitCode, long finishTimeMillis) { + super(exitCode, finishTimeMillis); } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java index d33fefa3730c11..43f006d9196458 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/TestCommand.java @@ -166,10 +166,7 @@ private BlazeCommandResult doTest( : buildResult.getDetailedExitCode(); env.getEventBus() .post( - new TestingCompleteEvent( - detailedExitCode.getExitCode(), - buildResult.getStopTime(), - buildResult.getWasSuspended())); + new TestingCompleteEvent(detailedExitCode.getExitCode(), buildResult.getStopTime())); return BlazeCommandResult.detailedExitCode(detailedExitCode); } // TODO(bazel-team): the check above shadows NO_TESTS_FOUND, but switching the conditions breaks @@ -188,11 +185,7 @@ private BlazeCommandResult doTest( .build()) : buildResult.getDetailedExitCode(); env.getEventBus() - .post( - new NoTestsFound( - detailedExitCode.getExitCode(), - buildResult.getStopTime(), - buildResult.getWasSuspended())); + .post(new NoTestsFound(detailedExitCode.getExitCode(), buildResult.getStopTime())); return BlazeCommandResult.detailedExitCode(detailedExitCode); } @@ -211,11 +204,7 @@ private BlazeCommandResult doTest( DetailedExitCode.DetailedExitCodeComparator.chooseMoreImportantWithFirstIfTie( buildResult.getDetailedExitCode(), testResults); env.getEventBus() - .post( - new TestingCompleteEvent( - detailedExitCode.getExitCode(), - buildResult.getStopTime(), - buildResult.getWasSuspended())); + .post(new TestingCompleteEvent(detailedExitCode.getExitCode(), buildResult.getStopTime())); return BlazeCommandResult.detailedExitCode(detailedExitCode); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/info/ServerPidInfoItem.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/info/ServerPidInfoItem.java index ac8e904d410a3d..3e3e710f5b64fc 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/info/ServerPidInfoItem.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/info/ServerPidInfoItem.java @@ -18,7 +18,6 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.runtime.InfoItem; -import com.google.devtools.build.lib.util.ProcessUtils; /** Info item for server_pid. */ public final class ServerPidInfoItem extends InfoItem { @@ -28,6 +27,6 @@ public ServerPidInfoItem(String productName) { @Override public byte[] get(Supplier configurationSupplier, CommandEnvironment env) { - return print(ProcessUtils.getpid()); + return print(ProcessHandle.current().pid()); } } diff --git a/src/main/native/unix_jni.cc b/src/main/native/unix_jni.cc index fa01f353a2a775..ba13392e9659d9 100644 --- a/src/main/native/unix_jni.cc +++ b/src/main/native/unix_jni.cc @@ -29,6 +29,9 @@ #include #include +// Linting disabled for this line because for google code we could use +// absl::Mutex but we cannot yet because Bazel doesn't depend on absl. +#include // NOLINT #include #include @@ -44,6 +47,20 @@ namespace blaze_jni { +static void PostException(JNIEnv *env, const char *exception_classname, + const std::string &message) { + jclass exception_class = env->FindClass(exception_classname); + bool success = false; + if (exception_class != nullptr) { + success = env->ThrowNew(exception_class, message.c_str()) == 0; + } + if (!success) { + fprintf(stderr, "error: Failure to throw java error: %s\n", + message.c_str()); + abort(); // panic! + } +} + // See unix_jni.h. void PostException(JNIEnv *env, int error_number, const std::string& message) { // Keep consistent with package-info.html! @@ -114,13 +131,12 @@ void PostException(JNIEnv *env, int error_number, const std::string& message) { default: exception_classname = "java/io/IOException"; } - jclass exception_class = env->FindClass(exception_classname); - if (exception_class != nullptr) { - env->ThrowNew(exception_class, - (message + " (" + ErrorMessage(error_number) + ")").c_str()); - } else { - abort(); // panic! - } + PostException(env, exception_classname, + message + " (" + ErrorMessage(error_number) + ")"); +} + +static void PostAssertionError(JNIEnv *env, const std::string& message) { + PostException(env, "java/lang/AssertionError", message); } // Throws RuntimeExceptions for IO operations which fail unexpectedly. @@ -163,6 +179,50 @@ static bool PostRuntimeException(JNIEnv *env, int error_number, } } +static JavaVM *GetJavaVM(JNIEnv *env) { + static JavaVM *java_vm = nullptr; + static std::mutex java_vm_mtx; + std::lock_guard lock(java_vm_mtx); + if (env != nullptr) { + JavaVM *env_java_vm; + jint value = env->GetJavaVM(&env_java_vm); + if (value != 0) { + return nullptr; + } + if (java_vm == nullptr) { + java_vm = env_java_vm; + } else if (java_vm != env_java_vm) { + return nullptr; + } + } + return java_vm; +} + +static void PerformIntegerValueCallback(jobject object, const char *callback, + int value) { + JavaVM *java_vm = GetJavaVM(nullptr); + JNIEnv *java_env; + int status = java_vm->GetEnv((void **)&java_env, JNI_VERSION_1_8); + bool attach_current_thread = false; + if (status == JNI_EDETACHED) { + attach_current_thread = true; + } else { + CHECK_EQ(status, JNI_OK); + } + if (attach_current_thread) { + CHECK_EQ(java_vm->AttachCurrentThread((void **)&java_env, nullptr), 0); + } + jclass clazz = java_env->GetObjectClass(object); + CHECK_NEQ(clazz, nullptr); + jmethodID method_id = java_env->GetMethodID(clazz, callback, "(I)V"); + CHECK_NEQ(method_id, nullptr); + java_env->CallVoidMethod(object, method_id, value); + + if (attach_current_thread) { + CHECK_EQ(java_vm->DetachCurrentThread(), JNI_OK); + } +} + // TODO(bazel-team): split out all the FileSystem class's native methods // into a separate source file, fsutils.cc. @@ -1206,15 +1266,43 @@ Java_com_google_devtools_build_lib_platform_SleepPreventionModule_00024SleepPrev return portable_pop_disable_sleep(); } +jobject g_suspend_module; + /* - * Class: com_google_devtools_build_lib_platform_SuspendCounter - * Method: suspendCountJNI - * Signature: ()I + * Class: Java_com_google_devtools_build_lib_platform_SystemSuspensionModule + * Method: registerJNI + * Signature: ()V */ -extern "C" JNIEXPORT jint JNICALL -Java_com_google_devtools_build_lib_platform_SuspendCounter_suspendCountJNI( - JNIEnv *, jclass) { - return portable_suspend_count(); +extern "C" JNIEXPORT void JNICALL +Java_com_google_devtools_build_lib_platform_SystemSuspensionModule_registerJNI( + JNIEnv *env, jobject local_object) { + + if (g_suspend_module != nullptr) { + PostAssertionError(env, + "Singleton SystemSuspensionModule already registered"); + return; + } + + JavaVM *java_vm = GetJavaVM(env); + if (java_vm == nullptr) { + PostAssertionError( + env, "Unable to get javaVM registering SystemSuspensionModule"); + return; + } + + g_suspend_module = env->NewGlobalRef(local_object); + if (g_suspend_module == nullptr) { + PostAssertionError( + env, "Unable to create global ref for SystemSuspensionModule"); + return; + } + portable_start_suspend_monitoring(); +} + +void suspend_callback(SuspensionReason value) { + if (g_suspend_module != nullptr) { + PerformIntegerValueCallback(g_suspend_module, "suspendCallback", value); + } } /* diff --git a/src/main/native/unix_jni.h b/src/main/native/unix_jni.h index 4138f8c9fa99d8..bc550d4a60f909 100644 --- a/src/main/native/unix_jni.h +++ b/src/main/native/unix_jni.h @@ -34,6 +34,9 @@ namespace blaze_jni { } \ } while (0) +#define CHECK_EQ(a, b) CHECK((a) == (b)) +#define CHECK_NEQ(a, b) CHECK((a) != (b)) + #if defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__) // stat64 is deprecated on OS X/BSD. typedef struct stat portable_stat_struct; @@ -105,9 +108,22 @@ int portable_sysctlbyname(const char *name_chars, void *mibp, size_t *sizep); int portable_push_disable_sleep(); int portable_pop_disable_sleep(); -// Returns the number of times that the process has been suspended (SIGSTOP, -// computer put to sleep, etc.) since Bazel started. -int portable_suspend_count(); +// Starts up any infrastructure needed to do suspend monitoring. +// May be called more than once. +void portable_start_suspend_monitoring(); + +// These need to be kept in sync with constants in +// j/c/g/devtools/build/lib/buildtool/buildevent/SystemSuspensionEvent.java +typedef enum { + SuspensionReasonSIGTSTP = 0, + SuspensionReasonSIGCONT = 1, + SuspensionReasonSleep = 2, + SuspensionReasonWake = 3 +} SuspensionReason; + +// Declaration for callback function that is called by suspend monitoring +// when a suspension is detected. +extern void suspend_callback(SuspensionReason value); // Returns the number of times that the system has received a memory pressure // warning notification since Bazel started. diff --git a/src/main/native/unix_jni_bsd.cc b/src/main/native/unix_jni_bsd.cc index c4166ff486a6b6..2548c2586ad52e 100644 --- a/src/main/native/unix_jni_bsd.cc +++ b/src/main/native/unix_jni_bsd.cc @@ -130,9 +130,8 @@ int portable_pop_disable_sleep() { return -1; } -int portable_suspend_count() { +void portable_start_suspend_monitoring() { // Currently not implemented. - return 0; } int portable_memory_pressure_warning_count() { diff --git a/src/main/native/unix_jni_darwin.cc b/src/main/native/unix_jni_darwin.cc index 2880a6b96d36db..adb24c675769bf 100644 --- a/src/main/native/unix_jni_darwin.cc +++ b/src/main/native/unix_jni_darwin.cc @@ -172,10 +172,7 @@ static os_log_t JniOSLog() { } while (0); // Protects all of the g_sleep_state_* statics. -// value is "leaked" intentionally because std::mutex is not trivially -// destructible at this time, g_sleep_state_mutex is a singleton, and -// leaking it has no consequences. -static std::mutex *g_sleep_state_mutex = new std::mutex; +static std::mutex g_sleep_state_mutex; // Keep track of our pushes and pops of sleep state. static int g_sleep_state_stack = 0; @@ -184,7 +181,7 @@ static int g_sleep_state_stack = 0; static IOPMAssertionID g_sleep_state_assertion = kIOPMNullAssertionID; int portable_push_disable_sleep() { - std::lock_guard lock(*g_sleep_state_mutex); + std::lock_guard lock(g_sleep_state_mutex); assert(g_sleep_state_stack >= 0); if (g_sleep_state_stack == 0) { assert(g_sleep_state_assertion == kIOPMNullAssertionID); @@ -201,7 +198,7 @@ int portable_push_disable_sleep() { } int portable_pop_disable_sleep() { - std::lock_guard lock(*g_sleep_state_mutex); + std::lock_guard lock(g_sleep_state_mutex); assert(g_sleep_state_stack > 0); g_sleep_state_stack -= 1; if (g_sleep_state_stack == 0) { @@ -217,10 +214,6 @@ int portable_pop_disable_sleep() { typedef struct { // Port used to relay sleep call back messages. io_connect_t connect_port; - - // Count of suspensions. Atomic because it can be read from any java thread - // and is written to from a dispatch_queue thread. - std::atomic_int suspend_count; } SuspendState; static void SleepCallBack(void *refcon, io_service_t service, @@ -234,41 +227,41 @@ static void SleepCallBack(void *refcon, io_service_t service, case kIOMessageSystemWillSleep: log_if_possible("suspend anomaly due to kIOMessageSystemWillSleep"); - ++state->suspend_count; + suspend_callback(SuspensionReasonSleep); // This needs to be acknowledged to allow sleep. IOAllowPowerChange(state->connect_port, (intptr_t)message_argument); break; - case kIOMessageSystemWillNotSleep: - log_if_possible( - "suspend anomaly cancelled due to kIOMessageSystemWillNotSleep"); - --state->suspend_count; - break; - - case kIOMessageSystemWillPowerOn: case kIOMessageSystemHasPoweredOn: - // We increment g_suspend_count when we are alerted to the sleep as - // opposed to when we wake up, because Macs have a "Dark Wake" mode (also - // known as PowerNap) which is when the processors (and disk and network) - // turn on for brief periods of time - // (https://support.apple.com/en-us/HT204032). Dark Wake does NOT trigger - // PowerOn messages through our sleep callbacks, but can allow + // Note that Macs have a "Dark Wake" mode (also known as PowerNap) which + // can have the processors (and disk and network) turn on. + // (https://support.apple.com/en-us/HT204032). Dark Wake does NOT + // trigger PowerOn messages through our sleep callbacks, but can allow // builds to proceed for a considerable amount of time (for example if // Time Machine is performing a back up). // There is currently a race condition where a build may finish // between the time we receive the kIOMessageSystemWillSleep and the - // machine actually goes to sleep (roughly 20 seconds in my experiments) - // or between the time we receive the kIOMessageSystemWillSleep and - // kIOMessageSystemWillNotSleep. This will result in us reporting that the - // build was suspended when it wasn't. I haven't come up with an smart way - // of avoiding this issue, but I don't think we really care. Over - // reporting "suspensions" is better than under reporting them. + // machine actually goes to sleep (roughly 20 seconds in my experiments). + // This will result in us reporting that the build was suspended when it + // wasn't. I haven't come up with an smart way of avoiding this issue, but + // I don't think we really care. Over reporting "suspensions" is better + // than under reporting them. + log_if_possible("suspend anomaly due to kIOMessageSystemHasPoweredOn"); + suspend_callback(SuspensionReasonWake); + break; + + case kIOMessageSystemWillPowerOn: + case kIOMessageSystemWillNotSleep: + // We don't handle will not sleep. This can only occur is somebody else + // cancels the sleep, and will never occur AFTER a + // kIOMessageSystemWillSleep. + // We don't handle will power on, we only care when it HAS powered on. default: break; } } -int portable_suspend_count() { +void portable_start_suspend_monitoring() { static dispatch_once_t once_token; static SuspendState suspend_state; dispatch_once(&once_token, ^{ @@ -279,15 +272,15 @@ int portable_suspend_count() { // Register to receive system sleep notifications. // Testing needs to be done manually. Use the logging to verify // that sleeps are being caught here. - // `log stream -level debug --predicate '(subsystem == "build.bazel")'` + // `/usr/bin/log \ + // stream -level debug --predicate '(subsystem == "build.bazel")'` suspend_state.connect_port = IORegisterForSystemPower( &suspend_state, ¬ifyPortRef, SleepCallBack, ¬ifierObject); CHECK(suspend_state.connect_port != MACH_PORT_NULL); IONotificationPortSetDispatchQueue(notifyPortRef, queue); // Register to deal with SIGCONT. - // We register for SIGCONT because we can't catch SIGSTOP and we can't - // distinguish a SIGCONT after a SIGSTOP from a SIGCONT after SIGTSTP. + // We register for SIGCONT because we can't catch SIGSTOP. // We do have the potential of "over counting" suspensions if you send // multiple SIGCONTs to a process without a previous SIGSTOP/SIGTSTP, // but there is no reason to send a SIGCONT without a SIGSTOP/SIGTSTP, and @@ -300,11 +293,19 @@ int portable_suspend_count() { CHECK(signal_source != nullptr); dispatch_source_set_event_handler(signal_source, ^{ log_if_possible("suspend anomaly due to SIGCONT"); - ++suspend_state.suspend_count; + suspend_callback(SuspensionReasonSIGCONT); + }); + dispatch_resume(signal_source); + signal_source = + dispatch_source_create(DISPATCH_SOURCE_TYPE_SIGNAL, SIGTSTP, 0, queue); + CHECK(signal_source != nullptr); + dispatch_source_set_event_handler(signal_source, ^{ + log_if_possible("suspend anomaly due to SIGTSTP"); + suspend_callback(SuspensionReasonSIGTSTP); }); dispatch_resume(signal_source); + log_if_possible("suspend monitoring registered"); }); - return suspend_state.suspend_count; } static std::atomic_int pressure_warning_count{0}; @@ -312,7 +313,7 @@ static std::atomic_int pressure_critical_count{0}; static void RegisterMemoryPressureHandler() { // To test use: - // log stream -level debug --predicate '(subsystem == "build.bazel")' + // /usr/bin/log stream -level debug --predicate '(subsystem == "build.bazel")' // sudo memory_pressure -S -l warn // sudo memory_pressure -S -l critical static dispatch_once_t once_token; @@ -334,6 +335,7 @@ static void RegisterMemoryPressureHandler() { } }); dispatch_resume(source); + log_if_possible("memory pressure handler registered"); }); } diff --git a/src/main/native/unix_jni_linux.cc b/src/main/native/unix_jni_linux.cc index e54c180a1c7055..f5043cee673a94 100644 --- a/src/main/native/unix_jni_linux.cc +++ b/src/main/native/unix_jni_linux.cc @@ -104,9 +104,8 @@ int portable_pop_disable_sleep() { return -1; } -int portable_suspend_count() { +void portable_start_suspend_monitoring() { // Currently not implemented. - return 0; } int portable_memory_pressure_warning_count() { diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD index 2588b13ee65b6b..edc23c2ce985e5 100644 --- a/src/main/native/windows/BUILD +++ b/src/main/native/windows/BUILD @@ -60,7 +60,7 @@ cc_binary( "memory_pressure_jni.cc", "processes-jni.cc", "sleep_prevention_jni.cc", - "suspend_counter_jni.cc", + "system_suspension_monitor_jni.cc", "//src/main/native:jni.h", "//src/main/native:jni_md.h", ], diff --git a/src/main/native/windows/suspend_counter_jni.cc b/src/main/native/windows/system_suspension_monitor_jni.cc similarity index 73% rename from src/main/native/windows/suspend_counter_jni.cc rename to src/main/native/windows/system_suspension_monitor_jni.cc index a6d2dbfde8dd15..c5763619cc7c83 100644 --- a/src/main/native/windows/suspend_counter_jni.cc +++ b/src/main/native/windows/system_suspension_monitor_jni.cc @@ -20,13 +20,12 @@ #include "src/main/native/jni.h" /* - * Class: com_google_devtools_build_lib_platform_SuspendCounter - * Method: suspendCountJNI - * Signature: ()I + * Class: Java_com_google_devtools_build_lib_platform_SystemSuspensionModule + * Method: registerJNI + * Signature: ()V */ -extern "C" JNIEXPORT jint JNICALL -Java_com_google_devtools_build_lib_platform_SuspendCounter_suspendCountJNI( - JNIEnv *, jclass) { +extern "C" JNIEXPORT void JNICALL +Java_com_google_devtools_build_lib_platform_SystemSuspensionModule_registerJNI( + JNIEnv *env, jobject local_object) { // Currently not implemented. - return 0; } diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java index 23262420cf2f84..721b4627bd9bcc 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java @@ -370,10 +370,7 @@ public void executeBuild(List targets) throws Exception { this.configurations = lastResult.getBuildConfigurationCollection(); finalizeBuildResult(lastResult); buildTool.stopRequest( - lastResult, - crash != null ? crash.getThrowable() : null, - detailedExitCode, - /*startSuspendCount=*/ 0); + lastResult, crash != null ? crash.getThrowable() : null, detailedExitCode); getSkyframeExecutor().notifyCommandComplete(reporter); if (crash != null) { runtime diff --git a/src/test/java/com/google/devtools/build/lib/platform/BUILD b/src/test/java/com/google/devtools/build/lib/platform/BUILD index ef706d8e69decb..945287d127c000 100644 --- a/src/test/java/com/google/devtools/build/lib/platform/BUILD +++ b/src/test/java/com/google/devtools/build/lib/platform/BUILD @@ -25,13 +25,17 @@ java_test( ) java_test( - name = "SuspendCounterTest", + name = "SystemSuspensionEventTest", timeout = "short", - srcs = ["SuspendCounterTest.java"], + srcs = ["SystemSuspensionEventTest.java"], deps = [ - "//src/main/java/com/google/devtools/build/lib/platform:suspend_counter", + "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/platform:system_suspension_module", "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/main/java/com/google/devtools/build/lib/util:process", + "//src/test/java/com/google/devtools/build/lib/analysis/util:util_internal", + "//src/test/java/com/google/devtools/build/lib/buildtool/util", + "//src/test/java/com/google/devtools/build/lib/packages:testutil", + "//third_party:guava", "//third_party:junit4", "//third_party:truth", ], diff --git a/src/test/java/com/google/devtools/build/lib/platform/SuspendCounterTest.java b/src/test/java/com/google/devtools/build/lib/platform/SuspendCounterTest.java deleted file mode 100644 index e68692e2b02e43..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/platform/SuspendCounterTest.java +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.platform; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.ProcessUtils; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link SuspendCounter}. */ -@RunWith(JUnit4.class) -public final class SuspendCounterTest { - - @Test - public void testSuspendCounter() throws Exception { - if (OS.getCurrent() == OS.DARWIN) { - int startSuspendCount = SuspendCounter.suspendCount(); - - // Send a SIGCONT to ourselves. - ProcessBuilder builder = - new ProcessBuilder("kill", "-s", "CONT", String.valueOf(ProcessUtils.getpid())); - Process process = builder.start(); - process.waitFor(); - - // Allow 10 seconds for signal to propagate. - for (int i = 0; i < 1000 && SuspendCounter.suspendCount() <= startSuspendCount; ++i) { - Thread.sleep(10 /* milliseconds */); - } - assertThat(SuspendCounter.suspendCount()).isGreaterThan(startSuspendCount); - } else { - assertThat(SuspendCounter.suspendCount()).isEqualTo(0); - } - } -} diff --git a/src/test/java/com/google/devtools/build/lib/platform/SystemSuspensionEventTest.java b/src/test/java/com/google/devtools/build/lib/platform/SystemSuspensionEventTest.java new file mode 100644 index 00000000000000..a7cd5169cd9537 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/platform/SystemSuspensionEventTest.java @@ -0,0 +1,76 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.platform; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.eventbus.Subscribe; +import com.google.devtools.build.lib.analysis.util.AnalysisMock; +import com.google.devtools.build.lib.buildtool.buildevent.SystemSuspensionEvent; +import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; +import com.google.devtools.build.lib.packages.util.MockGenruleSupport; +import com.google.devtools.build.lib.runtime.BlazeModule; +import com.google.devtools.build.lib.runtime.BlazeRuntime; +import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.util.OS; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link SystemSuspensionEvent}. */ +@RunWith(JUnit4.class) +public final class SystemSuspensionEventTest extends BuildIntegrationTestCase { + static class SystemSuspensionEventListener extends BlazeModule { + public int suspensionEventCount = 0; + + @Override + public void beforeCommand(CommandEnvironment env) { + env.getEventBus().register(this); + } + + @Subscribe + public void suspensionEvent(SystemSuspensionEvent event) { + ++suspensionEventCount; + } + } + + private final SystemSuspensionEventListener eventListener = new SystemSuspensionEventListener(); + + @Override + protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { + return super.getRuntimeBuilder() + .addBlazeModule(eventListener) + .addBlazeModule(new SystemSuspensionModule()); + } + + @Test + public void testSuspendCounter() throws Exception { + if (OS.getCurrent() == OS.DARWIN) { + AnalysisMock.get().setupMockToolsRepository(mockToolsConfig); + MockGenruleSupport.setup(mockToolsConfig); + StringBuilder buildFile = new StringBuilder(); + // Send a SIGCONT to ourselves which should cause our signal handler to fire. + buildFile + .append("genrule(name='signal', ") + .append("outs=['signal.out'], ") + .append("cmd='/bin/kill -s CONT ") + .append(ProcessHandle.current().pid()) + .append(" > $@')\n"); + write("system_suspension_event/BUILD", buildFile.toString()); + buildTarget("//system_suspension_event:signal"); + assertThat(eventListener.suspensionEventCount).isEqualTo(1); + } + } +} diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD index ac718a9e6e1cad..f50c7022a963ae 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD @@ -29,7 +29,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/sandbox", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/main/java/com/google/devtools/build/lib/util:process", "//src/main/java/com/google/devtools/build/lib/util:resource_converter", "//src/main/java/com/google/devtools/build/lib/util:single_line_formatter", "//src/main/java/com/google/devtools/build/lib/util/io", diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java index c6fd83cc4bca33..f5689e25fb6fc9 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/RemoteWorker.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.shell.CommandException; import com.google.devtools.build.lib.shell.CommandResult; import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.ProcessUtils; import com.google.devtools.build.lib.util.SingleLineFormatter; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.DigestHashFunction.DigestFunctionConverter; @@ -191,10 +190,10 @@ private void createPidFile() throws IOException { return; } - final Path pidFile = getFileSystem().getPath(workerOptions.pidFile); + Path pidFile = getFileSystem().getPath(workerOptions.pidFile); try (Writer writer = new OutputStreamWriter(pidFile.getOutputStream(), StandardCharsets.UTF_8)) { - writer.write(Integer.toString(ProcessUtils.getpid())); + writer.write(Long.toString(ProcessHandle.current().pid())); writer.write("\n"); }