message: Remove converted function self reference from its globals to avoid circular dependencies. Override the self argument with the results of inspect_utils.getmethodclass for consistency. Clean up the module cleanup mechanism.
diff: @@ -192,6 +192,7 @@ def converted_call(f, owner, options, *args, **kwargs):
  arg_map_target = f
  f_class = inspect_utils.getmethodclass(f)
  + # TODO(mdan): This may be more elegantly handled using __get__?
  if f_class is not None:
  # If this is a method call, it may or may not include self.
  #
  @@ -204,7 +205,10 @@ def converted_call(f, owner, options, *args, **kwargs):
  if owner is not None and (not args or args[0] is not owner):
  effective_args = (owner,) + args
  else:
  - effective_args = args
  + # Always override the self arg, because it might be different from
  + # what the method was bound to - see inspect_utils.getmethodclass.
  + assert args, 'Bound function call without self argument?'
  + effective_args = (f_class,) + args[1:]
  partial_types = (f_class,)
  else:
  effective_args = args
  @@ -255,28 +259,30 @@ def converted_call(f, owner, options, *args, **kwargs):
  optional_features=options.optional_features)
  result = converted_f(*effective_args, **kwargs)
  - # When converting a function, we write a tmp file and import it as a module.
  - # This leaks the module's closure. Once we've executed the converted_f module
  - # and there is no more code left to be executed, we can clean up the module.
  -
  - # TODO(mdan): Look into workarounds that don't suffer from refcount leaks.
  - # Possibly attach the closure as a regular closure cell, instead of relying on
  - # module globals.
  -
  - # If there are callables in the result, they will fail to find their closure
  - # when called, so only delete module if all returned types are not callable.
  - flat_results = nest.flatten(result)
  - if all(map(_is_not_callable, flat_results)):
  +
  + # The converted function's closure is simply inserted into the function's
  + # module __dict__. Since modules are permanently cached, that results in
  + # leaking the entire closure.
  + # Normally, it's not safe to delete the module because that may release said
  + # closure as well. However, in the case of converted_call we are certain the
  + # function will not be executed again, so the closure should no longer be
  + # needed so long as the function doesn't return any executable code.
  + # TODO(mdan): Attach the closure properly, using cells.
  + if all(map(_is_not_callable, nest.flatten(result))):
  del sys.modules[converted_f.__module__]
  return result
  def _is_not_callable(obj):
  - # TODO(brianklee): What happens if obj is a tensor wrapping a py_func?
  - return (isinstance(obj,
  - (int, float, complex, str, bool, np.ndarray, np.generic))
  - or tensor_util.is_tensor(obj))
  + # TODO(brianklee): Handle case when obj is a tensor dependent on a py_func.
  + if isinstance(obj, (int, float, complex, str, bool)):
  + return True
  + if isinstance(obj, (np.ndarray, np.generic)):
  + return True
  + if tensor_util.is_tensor(obj):
  + return True
  + return False
  # TODO(mdan): Rename: to_ops?
  @@ -357,6 +363,11 @@ def to_graph(e,
  if tf_inspect.isfunction(e):
  compiled.__defaults__ = e.__defaults__
  + if hasattr(compiled, '__globals__'):
  + # Remove self to avoid circular references. This will probably only work
  + # so long as the function is not reentrant.
  + del compiled.__globals__[name]
  +
  # Need this so the source_mapping attribute is available for the context
  # manager to access for runtime errors.
  #
  