FIX: Clobbering plugin files on startup is problematic

On startup, (including when starting a rails console) we manipule a
collection of plugin files. Writing these files is done in multiple
observable steps, which presents opportunities for race conditions and
causes temporary corruption.

This commit uses the write, fsync and rename trick to atomically
overwrite these files instead, but reads them first to avoid unnecessary
writes.

c457d3bf was a previous attempt to fix the same problem.
This commit is contained in:
Daniel Waterworth 2020-03-04 17:28:26 +00:00
parent a157f4aaaa
commit 83e649d08e
2 changed files with 42 additions and 7 deletions

View File

@ -45,6 +45,39 @@ module Discourse
logs.join("\n".freeze)
end
def self.atomic_write_file(destination, contents)
begin
return if File.read(destination) == contents
rescue Errno::ENOENT
end
FileUtils.mkdir_p(File.join(Rails.root, 'tmp'))
temp_destination = File.join(Rails.root, 'tmp', SecureRandom.hex)
File.open(temp_destination, "w") do |fd|
fd.write(contents)
fd.fsync()
end
File.rename(temp_destination, destination)
nil
end
def self.atomic_ln_s(source, destination)
begin
return if File.readlink(destination) == source
rescue Errno::ENOENT, Errno::EINVAL
end
FileUtils.mkdir_p(File.join(Rails.root, 'tmp'))
temp_destination = File.join(Rails.root, 'tmp', SecureRandom.hex)
execute_command('ln', '-s', source, temp_destination)
File.rename(temp_destination, destination)
nil
end
private
class CommandRunner

View File

@ -543,12 +543,11 @@ class Plugin::Instance
Discourse::Utils.execute_command('mkdir', '-p', target)
target << name.gsub(/\s/, "_")
# TODO a cleaner way of registering and unregistering
Discourse::Utils.execute_command('rm', '-f', target)
Discourse::Utils.execute_command('ln', '-s', public_data, target)
Discourse::Utils.atomic_ln_s(public_data, target)
end
ensure_directory(Plugin::Instance.js_path)
ensure_directory(js_file_path)
contents = []
handlebars_includes.each { |hb| contents << "require_asset('#{hb}')" }
@ -558,12 +557,15 @@ class Plugin::Instance
contents << (is_dir ? "depend_on('#{f}')" : "require_asset('#{f}')")
end
File.delete(js_file_path) if js_asset_exists?
if contents.present?
contents.insert(0, "<%")
contents << "%>"
write_asset(js_file_path, contents.join("\n"))
Discourse::Utils.atomic_write_file(js_file_path, contents.join("\n"))
else
begin
File.delete(js_file_path)
rescue Errno::ENOENT
end
end
end