From ef45175ecc9d5a649b112fdc156838aaff74bf72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Tue, 7 Dec 2021 17:04:52 +0200 Subject: [PATCH] Fixed race condition in _compress_template (#1086) compressor avoids rendering the same node multiple times by checking for a key in offline_manifest: ``` if key in offline_manifest: continue [... render template ...] offline_manifest[key] = result ``` Multiple _compress_template calls can run concurrently, creating a race condition: each checks for a key in offline manifest, then each proceeds to render template. Finally they try to save the same file at the same time, causing #1082. My proposed fix is to atomically * check if the manifest key exists * if it doesn't exist, set it to a placeholder value (None) So, in nutshell, the first "if" part becomes: ``` with offline_manifest_lock: if key in offline_manifest: continue offline_manifest[key] = None ``` I'm not sure where to store the lock, I've put it at the module level currently. Perhaps there's a way to avoid the lock entirely. --- compressor/management/commands/compress.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/compressor/management/commands/compress.py b/compressor/management/commands/compress.py index 2c250e066..6e0919f8c 100644 --- a/compressor/management/commands/compress.py +++ b/compressor/management/commands/compress.py @@ -2,6 +2,7 @@ import os import sys import concurrent.futures +from threading import Lock from collections import OrderedDict, defaultdict from fnmatch import fnmatch @@ -21,6 +22,7 @@ TemplateDoesNotExist) from compressor.utils import get_mod_func +offline_manifest_lock = Lock() class Command(BaseCommand): help = "Compress content outside of the request/response cycle" @@ -244,14 +246,22 @@ def _compress_template(offline_manifest, nodes, parser, template, errors): rendered = parser.render_nodelist(template, context, node) key = get_offline_hexdigest(rendered) - if key in offline_manifest: - continue + # Atomically check if the key exists in offline manifest. + # If it doesn't, set a placeholder key (None). This is to prevent + # concurrent _compress_template instances from rendering the + # same node, and then writing to the same file. + with offline_manifest_lock: + if key in offline_manifest: + continue + + offline_manifest[key] = None try: result = parser.render_node(template, context, node) except Exception as e: errors.append(CommandError("An error occurred during rendering %s: " "%s" % (template.template_name, smart_str(e)))) + del offline_manifest[key] return result = result.replace( settings.COMPRESS_URL, settings.COMPRESS_URL_PLACEHOLDER