Skip to content

Commit 6fbab67

Browse files
Moved kill_processes to sandbox destructor (#140)
The order of object destruction in Python is undefined, so it is not safe to call an external function inside a __del__ method. By moving the function body directly inside the destructor we can avoid None reference errors.
1 parent a23db06 commit 6fbab67

File tree

2 files changed

+34
-38
lines changed

2 files changed

+34
-38
lines changed

cwl_utils/sandboxjs.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from cwl_utils.errors import JavascriptException, WorkflowException
2929
from cwl_utils.loghandler import _logger
3030
from cwl_utils.types import CWLOutputType
31-
from cwl_utils.utils import kill_processes, singularity_supports_userns
31+
from cwl_utils.utils import singularity_supports_userns
3232

3333
default_timeout = 20
3434

@@ -101,7 +101,35 @@ def __init__(
101101

102102
def __del__(self) -> None:
103103
try:
104-
kill_processes(self.processes_to_kill)
104+
while self.processes_to_kill:
105+
process = self.processes_to_kill.popleft()
106+
if isinstance(process.args, MutableSequence):
107+
args = process.args
108+
else:
109+
args = [process.args]
110+
cidfile = [
111+
str(arg).split("=")[1] for arg in args if "--cidfile" in str(arg)
112+
]
113+
if cidfile: # Try to be nice
114+
try:
115+
with open(cidfile[0]) as inp_stream:
116+
p = subprocess.Popen( # nosec
117+
["docker", "kill", inp_stream.read()],
118+
shell=False, # nosec
119+
)
120+
try:
121+
p.wait(timeout=10)
122+
except subprocess.TimeoutExpired:
123+
p.kill()
124+
except FileNotFoundError:
125+
pass
126+
if process.stdin:
127+
process.stdin.close()
128+
try:
129+
process.wait(10)
130+
except subprocess.TimeoutExpired:
131+
pass
132+
process.kill()
105133
except TypeError:
106134
pass
107135

cwl_utils/utils.py

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@
66
import urllib.parse
77
import urllib.request
88
from copy import deepcopy
9+
10+
from ruamel.yaml.main import YAML
11+
from ruamel.yaml.parser import ParserError
12+
from ruamel.yaml.scanner import ScannerError
913
from typing import (
1014
Any,
11-
Deque,
1215
Dict,
1316
List,
1417
MutableMapping,
@@ -18,10 +21,6 @@
1821
Union,
1922
)
2023

21-
from ruamel.yaml.main import YAML
22-
from ruamel.yaml.parser import ParserError
23-
from ruamel.yaml.scanner import ScannerError
24-
2524
from cwl_utils.errors import MissingKeyField
2625
from cwl_utils.loghandler import _logger
2726

@@ -78,37 +77,6 @@ def bytes2str_in_dicts(
7877
return inp
7978

8079

81-
def kill_processes(
82-
processes_to_kill, # type: Deque[subprocess.Popen[str]]
83-
) -> None:
84-
while processes_to_kill:
85-
process = processes_to_kill.popleft()
86-
if isinstance(process.args, MutableSequence):
87-
args = process.args
88-
else:
89-
args = [process.args]
90-
cidfile = [str(arg).split("=")[1] for arg in args if "--cidfile" in str(arg)]
91-
if cidfile: # Try to be nice
92-
try:
93-
with open(cidfile[0]) as inp_stream:
94-
p = subprocess.Popen( # nosec
95-
["docker", "kill", inp_stream.read()], shell=False # nosec
96-
)
97-
try:
98-
p.wait(timeout=10)
99-
except subprocess.TimeoutExpired:
100-
p.kill()
101-
except FileNotFoundError:
102-
pass
103-
if process.stdin:
104-
process.stdin.close()
105-
try:
106-
process.wait(10)
107-
except subprocess.TimeoutExpired:
108-
pass
109-
process.kill()
110-
111-
11280
def load_linked_file(
11381
base_url: urllib.parse.ParseResult, link: str, is_import: bool = False
11482
) -> Tuple[Any, urllib.parse.ParseResult]:

0 commit comments

Comments
 (0)