Skip to content

Commit 8e58c62

Browse files
timfeltimfel
committed
fix reported OSError in subprocess execution
1 parent 1e3e5ef commit 8e58c62

File tree

3 files changed

+109
-87
lines changed

3 files changed

+109
-87
lines changed

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixSubprocessModuleBuiltins.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@
5959
import com.oracle.graal.python.builtins.objects.PNone;
6060
import com.oracle.graal.python.builtins.objects.bytes.BytesNodes;
6161
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
62+
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;
63+
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum.ErrorAndMessagePair;
6264
import com.oracle.graal.python.builtins.objects.function.PArguments;
6365
import com.oracle.graal.python.builtins.objects.list.PList;
6466
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
@@ -219,8 +221,9 @@ private synchronized int forkExec(PList args, @SuppressWarnings("unused") PList
219221
if (!(err instanceof WritableByteChannel)) {
220222
throw raise(PythonBuiltinClassType.OSError, "there was an error writing the fork_exec error to the error pipe");
221223
} else {
224+
ErrorAndMessagePair pair = OSErrorEnum.fromException(e);
222225
try {
223-
((WritableByteChannel) err).write(ByteBuffer.wrap(("SubprocessError:0:" + e.getMessage()).getBytes()));
226+
((WritableByteChannel) err).write(ByteBuffer.wrap(("OSError:" + Long.toHexString(pair.oserror.getNumber()) + ":" + pair.message).getBytes()));
224227
} catch (IOException e1) {
225228
}
226229
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/OSErrorEnum.java

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,25 @@
4040
*/
4141
package com.oracle.graal.python.builtins.objects.exception;
4242

43+
import java.io.FileNotFoundException;
44+
import java.io.IOException;
45+
import java.nio.channels.ClosedChannelException;
46+
import java.nio.channels.NonReadableChannelException;
47+
import java.nio.channels.NonWritableChannelException;
48+
import java.nio.file.AccessDeniedException;
49+
import java.nio.file.DirectoryNotEmptyException;
50+
import java.nio.file.FileAlreadyExistsException;
51+
import java.nio.file.FileSystemException;
52+
import java.nio.file.FileSystemLoopException;
53+
import java.nio.file.NoSuchFileException;
54+
import java.nio.file.NotDirectoryException;
55+
import java.nio.file.NotLinkException;
56+
import java.util.regex.Matcher;
57+
import java.util.regex.Pattern;
58+
59+
import com.oracle.truffle.api.CompilerDirectives;
4360
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
61+
import com.oracle.truffle.api.CompilerDirectives.ValueType;
4462

4563
public enum OSErrorEnum {
4664

@@ -50,7 +68,7 @@ public enum OSErrorEnum {
5068
* <pre>
5169
* awk -F'\\s+' '/#define/ { printf("%s(%s, \"",$2,$3); for (i=5; i<=NF-1; i++) {if(i==5){printf("%s",$i);}else{printf("%s%s",sep,$i);} sep=OFS}; print "\")," }' /usr/include/asm-generic/errno*
5270
* </pre>
53-
*
71+
*
5472
* Manually are changed EWOULDBLOCK and EDEADLOCK and move before appropriate errors with the
5573
* same number, because ErrnoModuleBuiltins and PosixModuleBuiltins built a dictionary from this
5674
* enum.
@@ -222,11 +240,93 @@ public static OSErrorEnum fromMessage(String message) {
222240
@TruffleBoundary
223241
public static OSErrorEnum fromNumber(int number) {
224242
OSErrorEnum[] values = values();
225-
for (int i = number; i < values.length; i++) {
243+
for (int i = 0; i < values.length; i++) {
226244
if (values[i].getNumber() == number) {
227245
return values[i];
228246
}
229247
}
230248
return null;
231249
}
250+
251+
public static ErrorAndMessagePair fromException(Exception e) {
252+
if (e instanceof IOException) {
253+
if (e instanceof NoSuchFileException || e instanceof FileNotFoundException) {
254+
return new ErrorAndMessagePair(OSErrorEnum.ENOENT, OSErrorEnum.ENOENT.getMessage());
255+
} else if (e instanceof AccessDeniedException) {
256+
return new ErrorAndMessagePair(OSErrorEnum.EACCES, OSErrorEnum.EACCES.getMessage());
257+
} else if (e instanceof FileAlreadyExistsException) {
258+
return new ErrorAndMessagePair(OSErrorEnum.EEXIST, OSErrorEnum.EEXIST.getMessage());
259+
} else if (e instanceof NotDirectoryException) {
260+
return new ErrorAndMessagePair(OSErrorEnum.ENOTDIR, OSErrorEnum.ENOTDIR.getMessage());
261+
} else if (e instanceof DirectoryNotEmptyException) {
262+
return new ErrorAndMessagePair(OSErrorEnum.ENOTEMPTY, OSErrorEnum.ENOTEMPTY.getMessage());
263+
} else if (e instanceof FileSystemLoopException) {
264+
return new ErrorAndMessagePair(OSErrorEnum.ELOOP, OSErrorEnum.ELOOP.getMessage());
265+
} else if (e instanceof NotLinkException) {
266+
return new ErrorAndMessagePair(OSErrorEnum.EINVAL, OSErrorEnum.EINVAL.getMessage());
267+
} else if (e instanceof ClosedChannelException) {
268+
return new ErrorAndMessagePair(OSErrorEnum.EPIPE, OSErrorEnum.EPIPE.getMessage());
269+
} else if (e instanceof FileSystemException) {
270+
String reason = getReason((FileSystemException) e);
271+
OSErrorEnum oserror = OSErrorEnum.fromMessage(reason);
272+
if (oserror == null) {
273+
return new ErrorAndMessagePair(OSErrorEnum.EIO, reason);
274+
} else {
275+
return new ErrorAndMessagePair(oserror, oserror.getMessage());
276+
}
277+
} else { // Generic IOException
278+
OSErrorEnum oserror = tryFindErrnoFromMessage(e);
279+
if (oserror == null) {
280+
return new ErrorAndMessagePair(OSErrorEnum.EIO, getMessage(e));
281+
} else {
282+
return new ErrorAndMessagePair(oserror, oserror.getMessage());
283+
}
284+
}
285+
} else if (e instanceof SecurityException) {
286+
return new ErrorAndMessagePair(OSErrorEnum.EPERM, OSErrorEnum.EPERM.getMessage());
287+
} else if (e instanceof IllegalArgumentException) {
288+
return new ErrorAndMessagePair(OSErrorEnum.EINVAL, OSErrorEnum.EINVAL.getMessage());
289+
} else if (e instanceof UnsupportedOperationException) {
290+
return new ErrorAndMessagePair(OSErrorEnum.EOPNOTSUPP, OSErrorEnum.EOPNOTSUPP.getMessage());
291+
} else if (e instanceof NonReadableChannelException || e instanceof NonWritableChannelException) {
292+
return new ErrorAndMessagePair(OSErrorEnum.EBADF, OSErrorEnum.EBADF.getMessage());
293+
} else if (e instanceof RuntimeException) {
294+
throw (RuntimeException) e;
295+
} else {
296+
CompilerDirectives.transferToInterpreterAndInvalidate();
297+
throw new RuntimeException(getMessage(e), e);
298+
}
299+
}
300+
301+
private static final Pattern ERRNO_PATTERN = Pattern.compile("error=(\\d+)");
302+
303+
@TruffleBoundary
304+
private static String getMessage(Exception e) {
305+
return e.getMessage();
306+
}
307+
308+
@TruffleBoundary
309+
private static String getReason(FileSystemException e) {
310+
return e.getReason();
311+
}
312+
313+
@TruffleBoundary
314+
private static OSErrorEnum tryFindErrnoFromMessage(Exception e) {
315+
Matcher m = ERRNO_PATTERN.matcher(e.getMessage());
316+
if (m.find()) {
317+
return fromNumber(Integer.parseInt(m.group(1)));
318+
}
319+
return null;
320+
}
321+
322+
@ValueType
323+
public static final class ErrorAndMessagePair {
324+
public final OSErrorEnum oserror;
325+
public final String message;
326+
327+
public ErrorAndMessagePair(OSErrorEnum oserror, String message) {
328+
this.oserror = oserror;
329+
this.message = message;
330+
}
331+
}
232332
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PRaiseOSErrorNode.java

Lines changed: 3 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -40,33 +40,17 @@
4040
*/
4141
package com.oracle.graal.python.nodes;
4242

43-
import java.io.FileNotFoundException;
44-
import java.io.IOException;
45-
import java.nio.channels.ClosedChannelException;
46-
import java.nio.channels.NonReadableChannelException;
47-
import java.nio.channels.NonWritableChannelException;
48-
import java.nio.file.AccessDeniedException;
49-
import java.nio.file.DirectoryNotEmptyException;
50-
import java.nio.file.FileAlreadyExistsException;
51-
import java.nio.file.FileSystemException;
52-
import java.nio.file.FileSystemLoopException;
53-
import java.nio.file.NoSuchFileException;
54-
import java.nio.file.NotDirectoryException;
55-
import java.nio.file.NotLinkException;
56-
import java.util.regex.Matcher;
57-
import java.util.regex.Pattern;
58-
5943
import com.oracle.graal.python.PythonLanguage;
6044
import com.oracle.graal.python.builtins.PythonBuiltinClassType;
6145
import com.oracle.graal.python.builtins.objects.PNone;
6246
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum;
47+
import com.oracle.graal.python.builtins.objects.exception.OSErrorEnum.ErrorAndMessagePair;
6348
import com.oracle.graal.python.builtins.objects.exception.PBaseException;
6449
import com.oracle.graal.python.builtins.objects.function.PKeyword;
6550
import com.oracle.graal.python.nodes.call.special.CallVarargsMethodNode;
6651
import com.oracle.graal.python.runtime.PythonContext;
6752
import com.oracle.graal.python.runtime.PythonCore;
6853
import com.oracle.graal.python.runtime.exception.PException;
69-
import com.oracle.truffle.api.CompilerDirectives;
7054
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
7155
import com.oracle.truffle.api.dsl.Cached;
7256
import com.oracle.truffle.api.dsl.CachedContext;
@@ -81,8 +65,6 @@
8165
@ImportStatic(PGuards.class)
8266
public abstract class PRaiseOSErrorNode extends Node {
8367

84-
private static final Pattern ERRNO_PATTERN = Pattern.compile("error=(\\d+)");
85-
8668
public abstract PException execute(Frame frame, Object[] arguments);
8769

8870
public final PException raiseOSError(Frame frame, OSErrorEnum oserror) {
@@ -98,20 +80,6 @@ private static String getMessage(Exception e) {
9880
return e.getMessage();
9981
}
10082

101-
@TruffleBoundary
102-
private static String getReason(FileSystemException e) {
103-
return e.getReason();
104-
}
105-
106-
@TruffleBoundary
107-
private static OSErrorEnum tryFindErrnoFromMessage(Exception e) {
108-
Matcher m = ERRNO_PATTERN.matcher(e.getMessage());
109-
if (m.find()) {
110-
return OSErrorEnum.fromNumber(Integer.parseInt(m.group(1)));
111-
}
112-
return null;
113-
}
114-
11583
public final PException raiseOSError(Frame frame, OSErrorEnum oserror, String filename) {
11684
return execute(frame, new Object[]{oserror.getNumber(), oserror.getMessage(), filename});
11785
}
@@ -129,57 +97,8 @@ public final PException raiseOSError(Frame frame, Exception e, String filename)
12997
}
13098

13199
public final PException raiseOSError(Frame frame, Exception e, String filename, String filename2) {
132-
OSErrorEnum oserror;
133-
String message = null;
134-
if (e instanceof IOException) {
135-
if (e instanceof NoSuchFileException || e instanceof FileNotFoundException) {
136-
oserror = OSErrorEnum.ENOENT;
137-
} else if (e instanceof AccessDeniedException) {
138-
oserror = OSErrorEnum.EACCES;
139-
} else if (e instanceof FileAlreadyExistsException) {
140-
oserror = OSErrorEnum.EEXIST;
141-
} else if (e instanceof NotDirectoryException) {
142-
oserror = OSErrorEnum.ENOTDIR;
143-
} else if (e instanceof DirectoryNotEmptyException) {
144-
oserror = OSErrorEnum.ENOTEMPTY;
145-
} else if (e instanceof FileSystemLoopException) {
146-
oserror = OSErrorEnum.ELOOP;
147-
} else if (e instanceof NotLinkException) {
148-
oserror = OSErrorEnum.EINVAL;
149-
} else if (e instanceof ClosedChannelException) {
150-
oserror = OSErrorEnum.EPIPE;
151-
} else if (e instanceof FileSystemException) {
152-
String reason = getReason((FileSystemException) e);
153-
oserror = OSErrorEnum.fromMessage(reason);
154-
if (oserror == null) {
155-
oserror = OSErrorEnum.EIO;
156-
message = reason;
157-
}
158-
} else { // Generic IOException
159-
oserror = tryFindErrnoFromMessage(e);
160-
if (oserror == null) {
161-
oserror = OSErrorEnum.EIO;
162-
message = getMessage(e);
163-
}
164-
}
165-
} else if (e instanceof SecurityException) {
166-
oserror = OSErrorEnum.EPERM;
167-
} else if (e instanceof IllegalArgumentException) {
168-
oserror = OSErrorEnum.EINVAL;
169-
} else if (e instanceof UnsupportedOperationException) {
170-
oserror = OSErrorEnum.EOPNOTSUPP;
171-
} else if (e instanceof NonReadableChannelException || e instanceof NonWritableChannelException) {
172-
oserror = OSErrorEnum.EBADF;
173-
} else if (e instanceof RuntimeException) {
174-
throw (RuntimeException) e;
175-
} else {
176-
CompilerDirectives.transferToInterpreterAndInvalidate();
177-
throw new RuntimeException(getMessage(e), e);
178-
}
179-
if (message == null) {
180-
message = oserror.getMessage();
181-
}
182-
return execute(frame, new Object[]{oserror.getNumber(), message, (filename != null) ? filename : PNone.NONE, PNone.NONE, (filename2 != null) ? filename2 : PNone.NONE});
100+
ErrorAndMessagePair errorAndMessage = OSErrorEnum.fromException(e);
101+
return execute(frame, new Object[]{errorAndMessage.oserror.getNumber(), errorAndMessage.message, (filename != null) ? filename : PNone.NONE, PNone.NONE, (filename2 != null) ? filename2 : PNone.NONE});
183102
}
184103

185104
@Specialization

0 commit comments

Comments
 (0)