Guard against wrapping an already-wrapped callable

line_profiler/line_profiler.py::LineProfiler
    add_property()
        Rolled back into `add_callable()`, no longer a standalone method
    add_callable()
        - Now returning 1 if a function is added to the profiler, and 0
          otherwise
        - Now checking against adding the wrapper callable around an
          already-added callable

line_profiler/profiler_mixin.py::ByCountProfilerMixin
    wrap_{function,coroutine,[async_]generator}()
        - Now checking if `func` is a wrapper around an already-added
          callable, and returns the same wrapper without rewrapping if
          that is the case
        - Now marking wrappers so that they can be recognized and not
          rewrapped

tests/test_line_profiler.py
    test_double_decoration()
        New test that double-wrapping doesn't erroneously profile the
        wrapper instead of the intended callable
    test_property_decorator()
        Updated because we are no longer double-adding the property
        getter

Author: TTsangSC

line_profiler/line_profiler.py

@@ -28,6 +28,8 @@
 # NOTE: This needs to be in sync with ../kernprof.py and __init__.py
 __version__ = '4.3.0'
 
+is_function = inspect.isfunction
+
 
 def load_ipython_extension(ip):
     """ API for IPython to recognize this module as an IPython extension.
@@ -36,6 +38,34 @@ def load_ipython_extension(ip):
     ip.register_magics(LineProfilerMagics)
 
 
+def _get_underlying_functions(func):
+    """
+    Get the underlying function objects of a callable or an adjacent
+    object.
+
+    Returns:
+        funcs (list[Callable])
+    """
+    if any(check(func)
+           for check in (is_boundmethod, is_classmethod, is_staticmethod)):
+        return _get_underlying_functions(func.__func__)
+    if any(check(func)
+           for check in (is_partial, is_partialmethod, is_cached_property)):
+        return _get_underlying_functions(func.func)
+    if is_property(func):
+        result = []
+        for impl in func.fget, func.fset, func.fdel:
+            if impl is not None:
+                result.extend(_get_underlying_functions(impl))
+        return result
+    if not callable(func):
+        raise TypeError(f'func = {func!r}: '
+                        f'cannot get functions from {type(func)} objects')
+    if is_function(func):
+        return [func]
+    return [type(func).__call__]
+
+
 class LineProfiler(CLineProfiler, ByCountProfilerMixin):
     """
     A profiler that records the execution times of individual lines.
@@ -60,35 +90,30 @@ def __call__(self, func):
         start the profiler on function entry and stop it on function
         exit.
         """
+        # Note: if `func` is a `types.FunctionType` which is already
+        # decorated by the profiler, the same object is returned;
+        # otherwise, wrapper objects are always returned.
         self.add_callable(func)
         return self.wrap_callable(func)
 
     def add_callable(self, func):
         """
         Register a function, method, property, partial object, etc. with
         the underlying Cython profiler.
-        """
-        if is_property(func):
-            self.add_property(func)
-        elif any(check(func)
-                 for check in (is_boundmethod,
-                               is_classmethod, is_staticmethod)):
-            self.add_function(func.__func__)
-        elif any(check(func) for check in (is_partial, is_partialmethod,
-                                           is_cached_property)):
-            self.add_function(func.func)
-        else:
-            self.add_function(func)
 
-    def add_property(self, func):
-        """
-        Register a `property`'s getter, setter, and deleter
-        implementations with the underlying Cython profiler.
+        Returns:
+            1 if any function is added to the profiler, 0 otherwise
         """
-        for impl in func.fget, func.fset, func.fdel:
-            if impl is None:
+        guard = self._already_wrapped
+
+        nadded = 0
+        for impl in _get_underlying_functions(func):
+            if guard(impl):
                 continue
             self.add_function(impl)
+            nadded += 1
+
+        return 1 if nadded else 0
 
     def dump_stats(self, filename):
         """ Dump a representation of the data to a file as a pickled LineStats
@@ -110,16 +135,16 @@ def print_stats(self, stream=None, output_unit=None, stripzeros=False,
     def add_module(self, mod):
         """ Add all the functions in a module and its classes.
         """
-        from inspect import isclass, isfunction
+        from inspect import isclass
 
         nfuncsadded = 0
         for item in mod.__dict__.values():
             if isclass(item):
                 for k, v in item.__dict__.items():
-                    if isfunction(v):
+                    if is_function(v):
                         self.add_function(v)
                         nfuncsadded += 1
-            elif isfunction(item):
+            elif is_function(item):
                 self.add_function(item)
                 nfuncsadded += 1
 

line_profiler/profiler_mixin.py

@@ -195,8 +195,12 @@ def wrap_cached_property(self, func):
 
     def wrap_async_generator(self, func):
         """
-        Wrap an async generator to profile it.
+        Wrap an async generator function to profile it.
         """
+        # Prevent double-wrap
+        if self._already_wrapped(func):
+            return func
+
         @functools.wraps(func)
         async def wrapper(*args, **kwds):
             g = func(*args, **kwds)
@@ -212,12 +216,16 @@ async def wrapper(*args, **kwds):
                     self.disable_by_count()
                 input_ = (yield item)
 
-        return wrapper
+        return self._mark_wrapped(wrapper)
 
     def wrap_coroutine(self, func):
         """
-        Wrap a Python 3.5 coroutine to profile it.
+        Wrap a coroutine function to profile it.
         """
+        # Prevent double-wrap
+        if self._already_wrapped(func):
+            return func
+
         @functools.wraps(func)
         async def wrapper(*args, **kwds):
             self.enable_by_count()
@@ -227,12 +235,16 @@ async def wrapper(*args, **kwds):
                 self.disable_by_count()
             return result
 
-        return wrapper
+        return self._mark_wrapped(wrapper)
 
     def wrap_generator(self, func):
         """
-        Wrap a generator to profile it.
+        Wrap a generator function to profile it.
         """
+        # Prevent double-wrap
+        if self._already_wrapped(func):
+            return func
+
         @functools.wraps(func)
         def wrapper(*args, **kwds):
             g = func(*args, **kwds)
@@ -248,11 +260,16 @@ def wrapper(*args, **kwds):
                     self.disable_by_count()
                 input_ = (yield item)
 
-        return wrapper
+        return self._mark_wrapped(wrapper)
 
     def wrap_function(self, func):
-        """ Wrap a function to profile it.
         """
+        Wrap a function to profile it.
+        """
+        # Prevent double-wrap
+        if self._already_wrapped(func):
+            return func
+
         @functools.wraps(func)
         def wrapper(*args, **kwds):
             self.enable_by_count()
@@ -261,7 +278,15 @@ def wrapper(*args, **kwds):
             finally:
                 self.disable_by_count()
             return result
-        return wrapper
+
+        return self._mark_wrapped(wrapper)
+
+    def _already_wrapped(self, func):
+        return getattr(func, self._profiler_wrapped_marker, None) == id(self)
+
+    def _mark_wrapped(self, func):
+        setattr(func, self._profiler_wrapped_marker, id(self))
+        return func
 
     def run(self, cmd):
         """ Profile a single executable statment in the main namespace.
@@ -295,3 +320,5 @@ def __enter__(self):
 
     def __exit__(self, *_, **__):
         self.disable_by_count()
+
+    _profiler_wrapped_marker = '__line_profiler_id__'

tests/test_line_profiler.py

@@ -102,6 +102,25 @@ def test_enable_disable():
     assert lp.last_time == {}
 
 
+def test_double_decoration():
+    """
+    Test that wrapping the same function twice does not result in
+    spurious profiling entries.
+    """
+    profile = LineProfiler()
+    f_wrapped = profile(f)
+    f_double_wrapped = profile(f_wrapped)
+    assert f_double_wrapped is f_wrapped
+
+    with check_timings(profile):
+        assert profile.enable_count == 0
+        value = f_wrapped(10)
+        assert profile.enable_count == 0
+        assert value == f(10)
+
+    assert len(profile.get_stats().timings) == 1
+
+
 def test_function_decorator():
     """
     Test for `LineProfiler.wrap_function()`.
@@ -398,27 +417,26 @@ def test_property_decorator():
     """
     profile = LineProfiler()
 
-    with pytest.warns(UserWarning,
-                      match='Adding a function with a __wrapped__ attribute'):
-        class Object:
-            def __init__(self, x: int) -> None:
-                self.x = x
+    class Object:
+        def __init__(self, x: int) -> None:
+            self.x = x
+
+        @profile
+        @property
+        def foo(self) -> int:
+            return self.x * 2
 
-            @profile
-            @property
-            def foo(self) -> int:
-                return self.x * 2
+        # The profiler sees both the setter and the already-wrapped
+        # getter here, but it shouldn't re-wrap the getter
 
-            @profile
-            @foo.setter
-            def foo(self, foo) -> None:
-                self.x = foo // 2
+        @profile
+        @foo.setter
+        def foo(self, foo) -> None:
+            self.x = foo // 2
 
     assert isinstance(Object.foo, property)
     assert profile.enable_count == 0
-    # XXX: should we try do remove duplicates? (getter added twice here)
-    # (that's also why there is the warning)
-    assert len(profile.functions) == 3
+    assert len(profile.functions) == 2
     obj = Object(3)
     assert obj.foo == 6  # Use getter
     obj.foo = 10  # Use setter