Skip to content

Commit

Permalink
reduce call stack depth in task runtime (#240)
Browse files Browse the repository at this point in the history
Task runtime's use of `copy.deepcopy` on values (`WDL.Value`) led to copying of AST nodes, since values refer back to the expressions (`WDL.Expr`) that evaluated to them. This could lead to Python `RecursionError` for excessive call stack depth (issue #239).

Exempts these references from deep-copying. Also increases the Python call stack depth limit in the main entry point.
  • Loading branch information
mlin committed Sep 18, 2019
1 parent a3f6537 commit ce370ac
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 2 deletions.
2 changes: 2 additions & 0 deletions WDL/CLI.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@


def main(args=None):
sys.setrecursionlimit(1_000_000) # permit as much call stack depth as OS can give us

parser = ArgumentParser()
parser.add_argument(
"--version",
Expand Down
6 changes: 5 additions & 1 deletion WDL/Value.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
from typing import Any, List, Optional, Tuple, Dict, Iterable, Union
import json
from . import Error, Type
from ._util import CustomDeepCopyMixin


class Base(ABC):
class Base(CustomDeepCopyMixin, ABC):
"""The abstract base class for WDL values"""

type: Type.Base
Expand All @@ -28,6 +29,9 @@ class Base(ABC):
from ``WDL.Expr.eval``
"""

# exempt type & expr from deep-copying since they're immutable
_shallow_copy_attrs: List[str] = ["expr", "type"]

def __init__(self, type: Type.Base, value: Any) -> None:
assert isinstance(type, Type.Base)
self.type = type
Expand Down
40 changes: 39 additions & 1 deletion WDL/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,23 @@
import os
import json
import logging
import copy
from time import sleep
from datetime import datetime
from typing import Tuple, Dict, Set, Iterable, List, TypeVar, Generic, Optional
from contextlib import contextmanager
from typing import (
Tuple,
Dict,
Set,
Iterable,
Iterator,
List,
TypeVar,
Generic,
Optional,
Callable,
Any,
)
import coloredlogs

__all__: List[str] = []
Expand Down Expand Up @@ -218,3 +232,27 @@ def install_coloredlogs(logger: logging.Logger) -> None:
level_styles=level_styles,
fmt=LOGGING_FORMAT,
)


class CustomDeepCopyMixin:
"""
Mixin class overrides __deepcopy__ to consult an internal list of attribute names to be merely
shallow-copied when the time comes. Useful for attributes referencing large, immutable data
structures.
Override class variable _shallow_copy_attrs to a list of the attribute names to be
shallow-copied.
"""

_shallow_copy_attrs: Optional[List[str]] = None

def __deepcopy__(self, memo: Dict[int, Any]) -> Any: # pyre-ignore
cls = self.__class__
cp = cls.__new__(cls)
memo[id(self)] = cp
for k in self._shallow_copy_attrs or []:
v = self.__dict__[k]
memo[id(v)] = v
for k, v in self.__dict__.items():
setattr(cp, k, copy.deepcopy(v, memo))
return cp
2 changes: 2 additions & 0 deletions tests/test_6workflowrun.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import docker
import signal
import time
import sys
from .context import WDL

class TestWorkflowRunner(unittest.TestCase):
Expand All @@ -14,6 +15,7 @@ def setUp(self):
self._dir = tempfile.mkdtemp(prefix="miniwdl_test_workflowrun_")

def _test_workflow(self, wdl:str, inputs = None, expected_exception: Exception = None):
sys.setrecursionlimit(180) # set artificially low in unit tests to detect excessive recursion (issue #239)
try:
with tempfile.NamedTemporaryFile(dir=self._dir, suffix=".wdl", delete=False) as outfile:
outfile.write(wdl.encode("utf-8"))
Expand Down

0 comments on commit ce370ac

Please sign in to comment.