[checkmk-commits] Check_MK Git: check_mk: FIX: Fixed locking problem of multisite pages related to user loading/saving

Module: check_mk Branch: master Commit: 6e0e57ff00b5b18db64f32f32f8641676d40cd11 URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=6e0e57ff00b5b1… Author: Lars Michelsen <lm(a)mathias-kettner.de> Date: Mon Jul 8 13:10:58 2013 +0200 FIX: Fixed locking problem of multisite pages related to user loading/saving --- ChangeLog | 5 +++-- web/htdocs/lib.py | 15 ++++++++++++--- web/htdocs/userdb.py | 20 +++++++++++++------- web/htdocs/wato.py | 10 +++++----- web/plugins/userdb/ldap.py | 2 +- 5 files changed, 34 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1bc80bb..4ad6df4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -56,13 +56,11 @@ Multisite: * User accounts can now be locked after a specified amount of auth failures (lock_on_logon_failures can be set to a number of tries) - * FIX: better error message in case of exception in SNMP handling * Column Perf-O-Meter is now sortable: it sorts after the *first* performance value. This might not always be the one you like, but its far better than nothing. * logwatch: Logwatch icon no longer uses notes_url * check_mk-if.pnp: fixed bug with pnp template on esx hosts without perfdata - * Fix: Inventory screen: Now shows custom checks * Inventory screen: Host inventory also displays its clustered services * Rules: Renamed "Ignored services" to "Disabled services" Renamed "Ignored checks" to "Disabled checks" @@ -78,6 +76,9 @@ * Added option to disable automatic userdb synchronizations in multisite * Implemented search forms for most data tables * New icons in view footers: export as CSV, export as JSON + * FIX: better error message in case of exception in SNMP handling + * FIX: Inventory screen: Now shows custom checks + * FIX: Fixed locking problem of multisite pages related to user loading/saving WATO: * Allow to configure check-/retry_interval in second precision diff --git a/web/htdocs/lib.py b/web/htdocs/lib.py index bb40374..8eab33a 100644 --- a/web/htdocs/lib.py +++ b/web/htdocs/lib.py @@ -279,14 +279,23 @@ def aquire_lock(path): return # No recursive locking fd = os.open(path, os.O_RDONLY) fcntl.flock(fd, fcntl.LOCK_EX) - g_aquired_locks.append(fd) + g_aquired_locks.append((path, fd)) g_locked_paths.append(path) +def release_lock(path): + if path not in g_locked_paths: + return # no unlocking needed + for lock_path, fd in g_aquired_locks: + if lock_path == path: + fcntl.flock(fd, fcntl.LOCK_UN) + fd.close() + g_aquired_locks.remove((lock_path, fd)) + g_locked_paths.remove(path) + def release_all_locks(): global g_aquired_locks, g_locked_paths - for fd in g_aquired_locks: + for path, fd in g_aquired_locks: os.close(fd) g_aquired_locks = [] g_locked_paths = [] - diff --git a/web/htdocs/userdb.py b/web/htdocs/userdb.py index 4abc5d6..95d3f4a 100644 --- a/web/htdocs/userdb.py +++ b/web/htdocs/userdb.py @@ -107,7 +107,7 @@ def new_user_template(connector_id): return new_user def create_non_existing_user(connector_id, username): - users = load_users() + users = load_users(lock = True) if username in users: return # User exists. Nothing to do... @@ -122,13 +122,13 @@ def user_locked(username): return users[username].get('locked', False) def on_succeeded_login(username): - users = load_users() + users = load_users(lock = True) if "num_failed" in users[username]: users[username]["num_failed"] = 0 save_users(users) def on_failed_login(username): - users = load_users() + users = load_users(lock = True) if username in users: if "num_failed" in users[username]: users[username]["num_failed"] += 1 @@ -165,13 +165,14 @@ def declare_user_attribute(name, vs, user_editable = True, permission = None): def get_user_attributes(): return user_attributes.items() -def load_users(): +def load_users(lock = False): filename = root_dir + "contacts.mk" # Make sure that the file exists without modifying it, *if* it exists. - # Note the lock will be released at end of page request automatically. + # Note: the lock will be released at end of page request automatically. file(filename, "a") - aquire_lock(filename) + if lock: + aquire_lock(filename) # First load monitoring contacts from Check_MK's world. If this is # the first time, then the file will be empty, which is no problem. @@ -394,6 +395,11 @@ def save_users(profiles): # if os.path.isdir(entry): # shutil.rmtree(entry) + # Release the lock to make other threads access possible again asap + # This lock is set by load_users() only in the case something is expected + # to be written (like during user syncs, wato, ...) + release_lock(root_dir + "contacts.mk") + # Call the users_saved hook hooks.call("users-saved", users) @@ -647,7 +653,7 @@ def general_page_hook(): # Create initial auth.serials file, same issue as auth.php above serials_file = '%s/auth.serials' % os.path.dirname(defaults.htpasswd_file) if not os.path.exists(serials_file) or os.path.getsize(serials_file) == 0: - save_users(load_users()) + save_users(load_users(lock = True)) # Hook function can be registered here to execute actions on a "regular" base without # user triggered action. This hook is called on each page load. diff --git a/web/htdocs/wato.py b/web/htdocs/wato.py index 1c7ab16..8d69a4f 100644 --- a/web/htdocs/wato.py +++ b/web/htdocs/wato.py @@ -5246,7 +5246,7 @@ def create_snapshot(): def factory_reset(): # Darn. What makes things complicated here is that we need to conserve htpasswd, # at least the account of the currently logged in user. - users = userdb.load_users() + users = userdb.load_users(lock = True) for id in users.keys(): if id != config.user_id: del users[id] @@ -7917,7 +7917,7 @@ def mode_users(phase): userdb.hook_sync(add_to_changelog = True) roles = userdb.load_roles() - users = filter_hidden_users(userdb.load_users()) + users = filter_hidden_users(userdb.load_users(lock = phase == 'action' and html.var('_delete'))) timeperiods = load_timeperiods() contact_groups = userdb.load_group_information().get("contact", {}) @@ -8044,7 +8044,7 @@ def mode_users(phase): def mode_edit_user(phase): - users = userdb.load_users() + users = userdb.load_users(lock = phase == 'action') userid = html.var("edit") # missing -> new user cloneid = html.var("clone") # Only needed in 'new' mode new = userid == None @@ -8744,7 +8744,7 @@ def save_roles(roles): # be renamed and are not handled here. If new_id is None, # the role is being deleted def rename_user_role(id, new_id): - users = userdb.load_users() + users = userdb.load_users(lock = True) for user in users.values(): if id in user["roles"]: user["roles"].remove(id) @@ -11518,7 +11518,7 @@ def page_user_profile(): success = None if html.has_var('_save') and html.check_transaction(): try: - users = userdb.load_users() + users = userdb.load_users(lock = True) # Profile edit (user options like language etc.) if config.may('general.edit_profile'): diff --git a/web/plugins/userdb/ldap.py b/web/plugins/userdb/ldap.py index 8f9826e..0535d56 100644 --- a/web/plugins/userdb/ldap.py +++ b/web/plugins/userdb/ldap.py @@ -716,7 +716,7 @@ def ldap_sync(add_to_changelog, only_username): # filt = '(%s=%s)' % (ldap_user_id_attr(), only_username) import wato - users = load_users() + users = load_users(lock = True) ldap_users = ldap_get_users() # Remove users which are controlled by this connector but can not be found in