)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"c039e1785f80e1f08f0bc8bb68ed45ca20aede12","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Identity plugin thread safety"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A common case is for Nova (or other service) to create an admin"},{"line_number":10,"context_line":"authentication from a CONF file and then have many greenlet threads that"},{"line_number":11,"context_line":"want to reuse that authentication. If a token expires then many threads"},{"line_number":12,"context_line":"all try and fetch a new token to use and can step over each other."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_5e3953d7","line":9,"updated":"2015-09-09 22:53:17.000000000","message":"suggest change admin to service.","commit_id":"f6772a991b4e3a9e66ffb476b8195b12694149c8"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"c039e1785f80e1f08f0bc8bb68ed45ca20aede12","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Identity plugin thread safety"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"A common case is for Nova (or other service) to create an admin"},{"line_number":10,"context_line":"authentication from a CONF file and then have many greenlet threads that"},{"line_number":11,"context_line":"want to reuse that authentication. If a token expires then many threads"},{"line_number":12,"context_line":"all try and fetch a new token to use and can step over each other."},{"line_number":13,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_de4b235e","line":10,"updated":"2015-09-09 22:53:17.000000000","message":"lowercase CONF or change to configuration","commit_id":"f6772a991b4e3a9e66ffb476b8195b12694149c8"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"c039e1785f80e1f08f0bc8bb68ed45ca20aede12","unresolved":false,"context_lines":[{"line_number":14,"context_line":"I was hoping for a way to put a lock in so that all plugins were thread"},{"line_number":15,"context_line":"safe however fixing it for identity plugins solves almost all real world"},{"line_number":16,"context_line":"situations and anyone doing non-identity plugins will have to manage"},{"line_number":17,"context_line":"threads themselves."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ib6487de7de638abc69660c851bd048a8ec177109"},{"line_number":20,"context_line":"Closes-Bug: #1493835"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"da20952f_be6f4fc3","line":17,"updated":"2015-09-09 22:53:17.000000000","message":"this needs to go in some documentation so that plugin writers know what to do.","commit_id":"f6772a991b4e3a9e66ffb476b8195b12694149c8"}],"keystoneclient/auth/identity/base.py":[{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"c039e1785f80e1f08f0bc8bb68ed45ca20aede12","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        # will try and fetch a new token at once, so we want to ensure that"},{"line_number":245,"context_line":"        # only one thread tries to actually fetch from keystone at once."},{"line_number":246,"context_line":"        with self._lock:"},{"line_number":247,"context_line":"            if self._needs_reauthenticate():"},{"line_number":248,"context_line":"                self.auth_ref \u003d self.get_auth_ref(session)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        return self.auth_ref"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_de3563a8","line":247,"updated":"2015-09-09 22:53:17.000000000","message":"I don\u0027t think what this is doing is safe... we get a lock that supposedly protects all the fields referenced by self._needs_reauthenticate(), which uses a bunch of properties like self.auth_ref, self.reauthenticate, etc., that are actually public, so applications or plugins could be messing with these fields without the lock held.\n\nMaybe this could be handled through documentation.","commit_id":"f6772a991b4e3a9e66ffb476b8195b12694149c8"},{"author":{"_account_id":7191,"name":"Jamie Lennox","email":"jamielennox@gmail.com","username":"jamielennox"},"change_message_id":"fb0a826b2dc1f2e9c9f8c88d28e8e953da05d848","unresolved":false,"context_lines":[{"line_number":244,"context_line":"        # will try and fetch a new token at once, so we want to ensure that"},{"line_number":245,"context_line":"        # only one thread tries to actually fetch from keystone at once."},{"line_number":246,"context_line":"        with self._lock:"},{"line_number":247,"context_line":"            if self._needs_reauthenticate():"},{"line_number":248,"context_line":"                self.auth_ref \u003d self.get_auth_ref(session)"},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"        return self.auth_ref"}],"source_content_type":"text/x-python","patch_set":1,"id":"da20952f_244a4e56","line":247,"in_reply_to":"da20952f_de3563a8","updated":"2015-09-10 00:19:24.000000000","message":"I\u0027m not so worried about the fields here nor the contents of _needs_reauthenticate. The big problem is get_auth_ref which actually compiles a token request out of the fields on the plugin, sends it to keystone, then constructs an AccessInfo based on the response. The checking validity and then fetching a new one once before assigning it on the object is the issue.\n\nI thought initially that we could put the lock inside the _needs_reauthenticate but that means you could have multiple threads queueing up waiting for there chance to fetch a token they\u0027ve already determined is invalid. \n\nMost parameters on the object are public and i don\u0027t think i\u0027m too concerned about the case where someone is trying to modify parameters at the same time that a plugin is trying to authenticate with keystone. Python gives us some protection there and the rest is a real failure of the user. You should construct a plugin once and then let it go.","commit_id":"f6772a991b4e3a9e66ffb476b8195b12694149c8"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"3e74457d3c100050558d90bafd61a5f8ffe6535a","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        \"\"\""},{"line_number":241,"context_line":"        # Hey Kids! Thread safety is important particularly in the case where"},{"line_number":242,"context_line":"        # a service is creating an admin style plugin that will then proceed"},{"line_number":243,"context_line":"        # to make calls from many threads. As a token expires all the threads"},{"line_number":244,"context_line":"        # will try and fetch a new token at once, so we want to ensure that"},{"line_number":245,"context_line":"        # only one thread tries to actually fetch from keystone at once."},{"line_number":246,"context_line":"        with self._lock:"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_47199587","line":243,"updated":"2015-09-15 09:17:26.000000000","message":"I don\u0027t agree with the use case, but i\u0027ll take some thread safety","commit_id":"556c1a6633931207370106478fa2d155fbffb126"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"6b79255233abb9938d7ada13eb14cca16f98c997","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        # a service is creating an admin style plugin that will then proceed"},{"line_number":243,"context_line":"        # to make calls from many threads. As a token expires all the threads"},{"line_number":244,"context_line":"        # will try and fetch a new token at once, so we want to ensure that"},{"line_number":245,"context_line":"        # only one thread tries to actually fetch from keystone at once."},{"line_number":246,"context_line":"        with self._lock:"},{"line_number":247,"context_line":"            if self._needs_reauthenticate():"},{"line_number":248,"context_line":"                self.auth_ref \u003d self.get_auth_ref(session)"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_a3de450f","line":245,"updated":"2015-09-15 19:43:40.000000000","message":"change \"keystone\" to \"the identity service\"","commit_id":"556c1a6633931207370106478fa2d155fbffb126"}]}