Skip to content

Commit 082d987

Browse files
larsrc-googlecopybara-github
authored andcommitted
Implement available() method for Windows subprocesses.
RELNOTES: None PiperOrigin-RevId: 351574909
1 parent 5b04895 commit 082d987

File tree

5 files changed

+136
-0
lines changed

5 files changed

+136
-0
lines changed

src/main/java/com/google/devtools/build/lib/windows/WindowsProcesses.java

+6
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ public static long createProcess(
7676
/** Returns an opaque identifier of stderr stream for the process. */
7777
public static native long getStderr(long process);
7878

79+
/**
80+
* Returns an estimate of the number of bytes available to read on the stream. Unlike {@link
81+
* InputStream#available()}, this returns 0 on closed or broken streams.
82+
*/
83+
public static native int streamBytesAvailable(long stream);
84+
7985
/**
8086
* Reads data from the stream into the given array. {@code stream} should come from {@link
8187
* #getStdout(long)} or {@link #getStderr(long)}.

src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocess.java

+14
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ private static final class ProcessInputStream extends InputStream {
6969
this.nativeStream = nativeStream;
7070
}
7171

72+
@Override
73+
public int available() throws IOException {
74+
if (nativeStream == WindowsProcesses.INVALID) {
75+
throw new IllegalStateException("Stream already closed");
76+
}
77+
78+
int result = WindowsProcesses.streamBytesAvailable(nativeStream);
79+
if (result == -1) {
80+
throw new IOException(WindowsProcesses.streamGetLastError(nativeStream));
81+
}
82+
83+
return result;
84+
}
85+
7286
@Override
7387
public int read() throws IOException {
7488
byte[] buf = new byte[1];

src/main/native/windows/processes-jni.cc

+58
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,33 @@ class NativeOutputStream {
9797

9898
void SetHandle(HANDLE handle) { handle_ = handle; }
9999

100+
jint StreamBytesAvailable(JNIEnv* env) {
101+
if (closed_.load() || handle_ == INVALID_HANDLE_VALUE) {
102+
error_ = L"";
103+
return 0;
104+
}
105+
106+
DWORD avail = 0;
107+
if (!::PeekNamedPipe(handle_, NULL, 0, NULL, &avail, NULL)) {
108+
// Check if either the other end closed the pipe or we did it with
109+
// NativeOutputStream.Close() . In the latter case, we'll get a "system
110+
// call interrupted" error.
111+
if (GetLastError() == ERROR_BROKEN_PIPE || closed_.load()) {
112+
error_ = L"";
113+
return 0;
114+
} else {
115+
DWORD err_code = GetLastError();
116+
error_ = bazel::windows::MakeErrorMessage(WSTR(__FILE__), __LINE__,
117+
L"nativeStreamBytesAvailable",
118+
L"", err_code);
119+
return -1;
120+
}
121+
} else {
122+
error_ = L"";
123+
}
124+
return avail;
125+
}
126+
100127
jint ReadStream(JNIEnv* env, jbyteArray java_bytes, jint offset,
101128
jint length) {
102129
JavaByteArray bytes(env, java_bytes);
@@ -210,6 +237,7 @@ class NativeProcess {
210237
}
211238
}
212239

240+
// Set up childs stdin pipe.
213241
{
214242
HANDLE pipe_read_h, pipe_write_h;
215243
if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0)) {
@@ -220,6 +248,14 @@ class NativeProcess {
220248
}
221249
stdin_process = pipe_read_h;
222250
stdin_ = pipe_write_h;
251+
252+
// "Our" end of the pipe must not be inherited by the child process
253+
if (!SetHandleInformation(pipe_write_h, HANDLE_FLAG_INHERIT, 0)) {
254+
DWORD err_code = GetLastError();
255+
error_ = bazel::windows::MakeErrorMessage(
256+
WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
257+
return false;
258+
}
223259
}
224260

225261
if (!stdout_is_stream) {
@@ -260,6 +296,13 @@ class NativeProcess {
260296
}
261297
stdout_.SetHandle(pipe_read_h);
262298
stdout_process = pipe_write_h;
299+
// "Our" end of the pipe must not be inherited by the child process
300+
if (!SetHandleInformation(pipe_read_h, HANDLE_FLAG_INHERIT, 0)) {
301+
DWORD err_code = GetLastError();
302+
error_ = bazel::windows::MakeErrorMessage(
303+
WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
304+
return false;
305+
}
263306
}
264307

265308
if (stderr_same_handle_as_stdout) {
@@ -314,6 +357,13 @@ class NativeProcess {
314357
}
315358
stderr_.SetHandle(pipe_read_h);
316359
stderr_process = pipe_write_h;
360+
// "Our" end of the pipe must not be inherited by the child process
361+
if (!SetHandleInformation(pipe_read_h, HANDLE_FLAG_INHERIT, 0)) {
362+
DWORD err_code = GetLastError();
363+
error_ = bazel::windows::MakeErrorMessage(
364+
WSTR(__FILE__), __LINE__, L"nativeCreateProcess", wpath, err_code);
365+
return false;
366+
}
317367
}
318368
return proc_.Create(
319369
wpath, bazel::windows::GetJavaWstring(env, java_argv_rest),
@@ -443,6 +493,14 @@ Java_com_google_devtools_build_lib_windows_WindowsProcesses_readStream(
443493
return stream->ReadStream(env, java_bytes, offset, length);
444494
}
445495

496+
extern "C" JNIEXPORT jint JNICALL
497+
Java_com_google_devtools_build_lib_windows_WindowsProcesses_streamBytesAvailable(
498+
JNIEnv* env, jclass clazz, jlong stream_long) {
499+
NativeOutputStream* stream =
500+
reinterpret_cast<NativeOutputStream*>(stream_long);
501+
return stream->StreamBytesAvailable(env);
502+
}
503+
446504
extern "C" JNIEXPORT jint JNICALL
447505
Java_com_google_devtools_build_lib_windows_WindowsProcesses_getExitCode(
448506
JNIEnv* env, jclass clazz, jlong process_long) {

src/test/java/com/google/devtools/build/lib/windows/WindowsProcessesTest.java

+38
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,44 @@ public void testPartialRead() throws Exception {
198198
assertThat(new String(two, UTF_8)).isEqualTo("LLO");
199199
}
200200

201+
@Test
202+
public void testAvailable_givesBytesFromLiveProcess() throws Exception {
203+
process =
204+
WindowsProcesses.createProcess(mockBinary, mockArgs("O-HELLOWRLD"), null, null, null, null);
205+
byte[] one = new byte[2];
206+
byte[] two = new byte[3];
207+
208+
long stdout = WindowsProcesses.getStdout(process);
209+
// Need to wait until the process has posted its data before we can check available()
210+
assertThat(readStdout(one, 0, 2)).isEqualTo(2);
211+
assertNoStreamError(stdout);
212+
assertThat(WindowsProcesses.streamBytesAvailable(stdout)).isEqualTo(7);
213+
assertNoStreamError(stdout);
214+
215+
assertThat(readStdout(two, 0, 3)).isEqualTo(3);
216+
assertNoStreamError(stdout);
217+
assertThat(WindowsProcesses.streamBytesAvailable(stdout)).isEqualTo(4);
218+
assertNoStreamError(stdout);
219+
220+
WindowsProcesses.closeStream(stdout);
221+
assertThat(WindowsProcesses.streamBytesAvailable(stdout)).isEqualTo(0);
222+
assertThat(WindowsProcesses.streamGetLastError(stdout)).isEmpty();
223+
224+
assertThat(new String(one, UTF_8)).isEqualTo("HE");
225+
assertThat(new String(two, UTF_8)).isEqualTo("LLO");
226+
}
227+
228+
@Test
229+
public void testAvailable_doesNotFailOnDeadProcess() throws Exception {
230+
process = WindowsProcesses.createProcess(mockBinary, mockArgs("X42"), null, null, null, null);
231+
long stdout = WindowsProcesses.getStdout(process);
232+
assertThat(WindowsProcesses.waitFor(process, -1)).isEqualTo(0);
233+
assertThat(WindowsProcesses.getExitCode(process)).isEqualTo(42);
234+
// Windows allows streams to be read after the process has died.
235+
assertThat(WindowsProcesses.streamBytesAvailable(stdout)).isAtLeast(0);
236+
assertThat(WindowsProcesses.streamGetLastError(stdout)).isEmpty();
237+
}
238+
201239
@Test
202240
public void testArrayOutOfBounds() throws Exception {
203241
process =

src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java

+20
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import static com.google.common.truth.Truth.assertThat;
1818
import static java.nio.charset.StandardCharsets.UTF_8;
19+
import static org.junit.Assert.assertThrows;
1920

2021
import com.google.common.collect.ImmutableList;
2122
import com.google.common.collect.ImmutableMap;
@@ -26,6 +27,7 @@
2627
import com.google.devtools.build.lib.util.OS;
2728
import com.google.devtools.build.runfiles.Runfiles;
2829
import java.io.File;
30+
import java.io.InputStream;
2931
import org.junit.After;
3032
import org.junit.Before;
3133
import org.junit.Test;
@@ -125,6 +127,24 @@ public void testSystemDriveIsSet() throws Exception {
125127
assertThat(new String(buf, UTF_8).trim()).isEqualTo("X:");
126128
}
127129

130+
@Test
131+
public void testStreamAvailable_zeroAfterClose() throws Exception {
132+
SubprocessBuilder subprocessBuilder = new SubprocessBuilder(WindowsSubprocessFactory.INSTANCE);
133+
subprocessBuilder.setWorkingDirectory(new File("."));
134+
subprocessBuilder.setArgv(ImmutableList.of(mockBinary, "-jar", mockSubprocess, "OHELLO"));
135+
process = subprocessBuilder.start();
136+
InputStream inputStream = process.getInputStream();
137+
// We don't know if the process has already written to the pipe
138+
assertThat(inputStream.available()).isAnyOf(0, 5);
139+
process.waitFor();
140+
// Windows allows streams to be read after the process has died.
141+
assertThat(inputStream.available()).isAnyOf(0, 5);
142+
inputStream.close();
143+
assertThrows(IllegalStateException.class, inputStream::available)
144+
.getMessage()
145+
.contains("Stream already closed");
146+
}
147+
128148
/**
129149
* An argument and its command-line-escaped counterpart.
130150
*

0 commit comments

Comments
 (0)