Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make LazyLoader thread safe #46641

Merged
merged 1 commit into from
Apr 3, 2018
Merged

Make LazyLoader thread safe #46641

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 60 additions & 52 deletions salt/loader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import inspect
import tempfile
import functools
import threading
import types
from collections import MutableMapping
from zipimport import zipimporter
Expand Down Expand Up @@ -1100,7 +1101,8 @@ def __init__(self,
self.disabled = set(self.opts.get('disable_{0}{1}'.format(
self.tag, '' if self.tag[-1] == 's' else 's'), []))

self.refresh_file_mapping()
self._lock = threading.RLock()
self._refresh_file_mapping()

super(LazyLoader, self).__init__() # late init the lazy loader
# create all of the import namespaces
Expand Down Expand Up @@ -1167,7 +1169,7 @@ def missing_fun_string(self, function_name):
else:
return '\'{0}\' __virtual__ returned False'.format(mod_name)

def refresh_file_mapping(self):
def _refresh_file_mapping(self):
'''
refresh the mapping of the FS on disk
'''
Expand Down Expand Up @@ -1285,15 +1287,16 @@ def clear(self):
'''
Clear the dict
'''
super(LazyLoader, self).clear() # clear the lazy loader
self.loaded_files = set()
self.missing_modules = {}
self.loaded_modules = {}
# if we have been loaded before, lets clear the file mapping since
# we obviously want a re-do
if hasattr(self, 'opts'):
self.refresh_file_mapping()
self.initial_load = False
with self._lock:
super(LazyLoader, self).clear() # clear the lazy loader
self.loaded_files = set()
self.missing_modules = {}
self.loaded_modules = {}
# if we have been loaded before, lets clear the file mapping since
# we obviously want a re-do
if hasattr(self, 'opts'):
self._refresh_file_mapping()
self.initial_load = False

def __prep_mod_opts(self, opts):
'''
Expand Down Expand Up @@ -1504,14 +1507,14 @@ def _load_module(self, name):
virtual_funcs_to_process = ['__virtual__'] + self.virtual_funcs
for virtual_func in virtual_funcs_to_process:
virtual_ret, module_name, virtual_err, virtual_aliases = \
self.process_virtual(mod, module_name)
self._process_virtual(mod, module_name)
if virtual_err is not None:
log.trace(
'Error loading %s.%s: %s',
self.tag, module_name, virtual_err
)

# if process_virtual returned a non-True value then we are
# if _process_virtual returned a non-True value then we are
# supposed to not process this module
if virtual_ret is not True and module_name not in self.missing_modules:
# If a module has information about why it could not be loaded, record it
Expand Down Expand Up @@ -1601,56 +1604,61 @@ def _load(self, key):
if not isinstance(key, six.string_types) or '.' not in key:
raise KeyError
mod_name, _ = key.split('.', 1)
if mod_name in self.missing_modules:
return True
# if the modulename isn't in the whitelist, don't bother
if self.whitelist and mod_name not in self.whitelist:
raise KeyError

def _inner_load(mod_name):
for name in self._iter_files(mod_name):
if name in self.loaded_files:
continue
# if we got what we wanted, we are done
if self._load_module(name) and key in self._dict:
return True
return False
with self._lock:
# It is possible that the key is in the dictionary after
# acquiring the lock due to another thread loading it.
if mod_name in self.missing_modules or key in self._dict:
return True
# if the modulename isn't in the whitelist, don't bother
if self.whitelist and mod_name not in self.whitelist:
raise KeyError

def _inner_load(mod_name):
for name in self._iter_files(mod_name):
if name in self.loaded_files:
continue
# if we got what we wanted, we are done
if self._load_module(name) and key in self._dict:
return True
return False

# try to load the module
ret = None
reloaded = False
# re-scan up to once, IOErrors or a failed load cause re-scans of the
# filesystem
while True:
try:
ret = _inner_load(mod_name)
if not reloaded and ret is not True:
self.refresh_file_mapping()
reloaded = True
# try to load the module
ret = None
reloaded = False
# re-scan up to once, IOErrors or a failed load cause re-scans of the
# filesystem
while True:
try:
ret = _inner_load(mod_name)
if not reloaded and ret is not True:
self._refresh_file_mapping()
reloaded = True
continue
break
except IOError:
if not reloaded:
self._refresh_file_mapping()
reloaded = True
continue
break
except IOError:
if not reloaded:
self.refresh_file_mapping()
reloaded = True
continue

return ret

def _load_all(self):
'''
Load all of them
'''
for name in self.file_mapping:
if name in self.loaded_files or name in self.missing_modules:
continue
self._load_module(name)
with self._lock:
for name in self.file_mapping:
if name in self.loaded_files or name in self.missing_modules:
continue
self._load_module(name)

self.loaded = True
self.loaded = True

def reload_modules(self):
self.loaded_files = set()
self._load_all()
with self._lock:
self.loaded_files = set()
self._load_all()

def _apply_outputter(self, func, mod):
'''
Expand All @@ -1661,7 +1669,7 @@ def _apply_outputter(self, func, mod):
if func.__name__ in outp:
func.__outputter__ = outp[func.__name__]

def process_virtual(self, mod, module_name, virtual_func='__virtual__'):
def _process_virtual(self, mod, module_name, virtual_func='__virtual__'):
'''
Given a loaded module and its default name determine its virtual name

Expand Down