make sure we strictly follow the pattern gil.release(true); try { ... } finally { gil.acquire(); ... }

Author: timfel

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PosixModuleBuiltins.java

@@ -410,8 +410,8 @@ private void execv(VirtualFrame frame, PosixPath path, Object argv, SequenceStor
 
             auditNode.audit("os.exec", path.originalObject, argv, PNone.NONE);
 
+            gil.release(true);
             try {
-                gil.release(true);
                 posixLib.execv(getPosixSupport(), path.value, opaqueArgs);
             } catch (PosixException e) {
                 gil.acquire();
@@ -498,11 +498,10 @@ int open(VirtualFrame frame, PosixPath path, int flags, int mode, int dirFd,
                         return posixLib.openat(getPosixSupport(), dirFd, path.value, fixedFlags, mode);
                     } catch (PosixException e) {
                         errorProfile.enter();
-                        gil.acquire();
                         if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
                             PythonContext.triggerAsyncActions(this);
-                            gil.release(true);
                         } else {
+                            gil.acquire(); // need GIL to construct OSError
                             throw raiseOSErrorFromPosixException(frame, e, path.originalObject);
                         }
                     }
@@ -580,9 +579,7 @@ public PBytes read(VirtualFrame frame, int fd, int length,
                     } catch (PosixException e) {
                         errorProfile.enter();
                         if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
-                            gil.acquire(); // need gil to trigger actions or construct OSError
                             PythonContext.triggerAsyncActions(this);
-                            gil.release(true); // continue read loop without gil
                         } else {
                             throw e;
                         }
@@ -629,9 +626,7 @@ public long write(int fd, byte[] data,
                     } catch (PosixException e) {
                         errorProfile.enter();
                         if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
-                            gil.acquire();
                             PythonContext.triggerAsyncActions(this);
-                            gil.release(true);
                         } else {
                             throw e;
                         }
@@ -1793,21 +1788,23 @@ PTuple waitpid(VirtualFrame frame, long pid, int options,
                         @CachedLibrary("getPosixSupport()") PosixSupportLibrary posixLib,
                         @Cached BranchProfile errorProfile) {
             gil.release(true);
-            while (true) {
-                try {
-                    long[] result = posixLib.waitpid(getPosixSupport(), pid, options);
-                    gil.acquire();
-                    return factory().createTuple(new Object[]{result[0], result[1]});
-                } catch (PosixException e) {
-                    errorProfile.enter();
-                    gil.acquire();
-                    if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
-                        PythonContext.triggerAsyncActions(this);
-                        gil.release(true);
-                    } else {
-                        throw raiseOSErrorFromPosixException(frame, e);
+            try {
+                while (true) {
+                    try {
+                        long[] result = posixLib.waitpid(getPosixSupport(), pid, options);
+                        return factory().createTuple(new Object[]{result[0], result[1]});
+                    } catch (PosixException e) {
+                        errorProfile.enter();
+                        if (e.getErrorCode() == OSErrorEnum.EINTR.getNumber()) {
+                            PythonContext.triggerAsyncActions(this);
+                        } else {
+                            gil.acquire();
+                            throw raiseOSErrorFromPosixException(frame, e);
+                        }
                     }
                 }
+            } finally {
+                gil.acquire();
             }
         }
     }
@@ -1959,9 +1956,9 @@ int system(PBytes command,
             // conversions for emulated backend because the bytes version after fsencode conversion
             // is subject to sys.audit.
             auditNode.audit("os.system", command);
+            byte[] bytes = toBytesNode.execute(command);
             gil.release(true);
             try {
-                byte[] bytes = toBytesNode.execute(command);
                 Object cmdOpaque = posixLib.createPathFromBytes(getPosixSupport(), bytes);
                 return posixLib.system(getPosixSupport(), cmdOpaque);
             } finally {

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/TimeModuleBuiltins.java

@@ -486,8 +486,8 @@ Object sleep(PythonModule self, long seconds,
             try {
                 doSleep(seconds, deadline);
             } finally {
-                dylib.put(self, TIME_SLEPT, nanoTime() - t + timeSlept(self));
                 gil.acquire();
+                dylib.put(self, TIME_SLEPT, nanoTime() - t + timeSlept(self));
             }
             PythonContext.triggerAsyncActions(this);
             return PNone.NONE;

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/socket/SocketBuiltins.java

@@ -207,8 +207,8 @@ Object connect(PSocket socket, PTuple address,
                         @Cached GilNode gil,
                         @Cached GetObjectArrayNode getObjectArrayNode) {
             Object[] hostAndPort = getObjectArrayNode.execute(address);
-            gil.release(true);
             try {
+                gil.release(true);
                 try {
                     doConnect(socket, hostAndPort);
                 } finally {
@@ -358,10 +358,10 @@ Object recv(VirtualFrame frame, PSocket socket, int bufsize, @SuppressWarnings("
             if (socket.getSocket() == null) {
                 throw raiseOSError(frame, OSErrorEnum.ENOTCONN);
             }
-            ByteBuffer readBytes = PythonUtils.allocateByteBuffer(bufsize);
-            gil.release(true);
             try {
+                gil.release(true);
                 try {
+                    ByteBuffer readBytes = PythonUtils.allocateByteBuffer(bufsize);
                     int length = SocketUtils.recv(this, socket, readBytes);
                     return factory().createBytes(PythonUtils.getBufferArray(readBytes), length);
                 } finally {
@@ -447,8 +447,8 @@ Object recvInto(VirtualFrame frame, PSocket socket, PByteArray buffer, @Suppress
             int bufferLen = lenNode.execute(storage);
             if (byteStorage.profile(storage instanceof ByteSequenceStorage)) {
                 ByteBuffer byteBuffer = ((ByteSequenceStorage) storage).getBufferView();
-                gil.release(true);
                 try {
+                    gil.release(true);
                     try {
                         return SocketUtils.recv(this, socket, byteBuffer);
                     } finally {
@@ -463,8 +463,8 @@ Object recvInto(VirtualFrame frame, PSocket socket, PByteArray buffer, @Suppress
                 byte[] targetBuffer = new byte[bufferLen];
                 ByteBuffer byteBuffer = PythonUtils.wrapByteBuffer(targetBuffer);
                 int length;
-                gil.release(true);
                 try {
+                    gil.release(true);
                     try {
                         length = SocketUtils.recv(this, socket, byteBuffer);
                     } finally {
@@ -521,8 +521,8 @@ Object send(VirtualFrame frame, PSocket socket, PBytes bytes, @SuppressWarnings(
             }
             int written;
             ByteBuffer buffer = PythonUtils.wrapByteBuffer(toBytes.execute(bytes.getSequenceStorage()));
-            gil.release(true);
             try {
+                gil.release(true);
                 try {
                     written = SocketUtils.send(this, socket, buffer);
                 } finally {
@@ -564,8 +564,8 @@ Object sendAll(VirtualFrame frame, PSocket socket, PBytesLike bytes, @SuppressWa
                     timeoutMillis = timeoutHelper.checkAndGetRemainingTimeout(this);
                 }
                 int written;
-                gil.release(true);
                 try {
+                    gil.release(true);
                     try {
                         written = SocketUtils.send(this, socket, buffer, timeoutMillis);
                     } finally {