uploads: fix temp files not being cleaned up quickly enough.

Fix temp files generated during the upload process not being cleaned up quickly enough. This included
downloaded files, generated preview images, and Ugoira video conversions.

Before we relied on `Tempfile` cleaning up files automatically. But this only happened when the
Tempfile object was garbage collected, which could take a long time. In the meantime we could have
hundreds of megabytes of temp files hanging around.

The fix is to explicitly close temp files when we're done with them. But the standard `Tempfile`
class doesn't immediately delete the file when it's closed. So we also have to introduce a
Danbooru::Tempfile wrapper that deletes the tempfile as soon as it's closed.
This commit is contained in:
evazion
2022-11-15 17:34:59 -06:00
parent 21a779455f
commit e935f01358
14 changed files with 67 additions and 27 deletions

View File

@@ -55,20 +55,21 @@ class BigqueryExportService
file = dump_records! file = dump_records!
upload_to_bigquery!(file) upload_to_bigquery!(file)
ensure
file&.close
end end
# Dump the table's records to a gzipped, newline-delimited JSON tempfile. # Dump the table's records to a gzipped, newline-delimited JSON tempfile.
def dump_records! def dump_records!(file = Danbooru::Tempfile.new("danbooru-export-dump-#{model.name}-", binmode: true))
file = Tempfile.new("danbooru-export-dump-", binmode: true) gzip = Zlib::GzipWriter.new(file)
file = Zlib::GzipWriter.new(file)
CurrentUser.scoped(User.anonymous) do CurrentUser.scoped(User.anonymous) do
records.find_each(batch_size: 5_000) do |record| records.find_each(batch_size: 5_000) do |record|
file.puts(record.to_json) gzip.puts(record.to_json)
end end
end end
file.close # flush zlib footer gzip.finish
file file
end end

View File

@@ -171,11 +171,11 @@ module Danbooru
# Download a file from `url` and return a {MediaFile}. # Download a file from `url` and return a {MediaFile}.
# #
# @param url [String] the URL to download # @param url [String] the URL to download
# @param file [Tempfile] the file to download the URL to # @param file [Danbooru::Tempfile] the file to download the URL to
# @raise [DownloadError] if the server returns a non-200 OK response # @raise [DownloadError] if the server returns a non-200 OK response
# @raise [FileTooLargeError] if the file exceeds Danbooru's maximum download size. # @raise [FileTooLargeError] if the file exceeds Danbooru's maximum download size.
# @return [Array<(HTTP::Response, MediaFile)>] the HTTP response and the downloaded file # @return [Array<(HTTP::Response, MediaFile)>] the HTTP response and the downloaded file
def download_media(url, file: Tempfile.new("danbooru-download-", binmode: true)) def download_media(url, file: Danbooru::Tempfile.new("danbooru-download-#{url.parameterize.truncate(96)}-", binmode: true))
response = get(url) response = get(url)
raise DownloadError, "#{url} failed with code #{response.status}" if response.status != 200 raise DownloadError, "#{url} failed with code #{response.status}" if response.status != 200

View File

@@ -0,0 +1,14 @@
# frozen_string_literal: true
# Like Tempfile, but delete the tempfile when it's closed.
#
# The Tempfile class in the standard library doesn't delete the file immediately when you call `file.close`. Instead you
# have to call `file.close!` or `file.unlink` to delete the file, or wait until the object gets garbage collected, which
# can take a long time. This makes it so that Tempfiles are cleaned up immediately on close.
module Danbooru
class Tempfile < ::Tempfile
def close(unlink = true)
super
end
end
end

View File

@@ -52,6 +52,8 @@ class DiscordSlashCommand
}, },
}] }]
} }
ensure
preview&.close
end end
def get_last_message_with_url(limit: 10) def get_last_message_with_url(limit: 10)

View File

@@ -22,7 +22,7 @@ class FFmpeg
# #
# @return [MediaFile] the preview image # @return [MediaFile] the preview image
def smart_video_preview def smart_video_preview
vp = Tempfile.new(["video-preview", ".png"], binmode: true) vp = Danbooru::Tempfile.new(["danbooru-video-preview-#{file.md5}-", ".png"], binmode: true)
# https://ffmpeg.org/ffmpeg.html#Main-options # https://ffmpeg.org/ffmpeg.html#Main-options
# https://ffmpeg.org/ffmpeg-filters.html#thumbnail # https://ffmpeg.org/ffmpeg-filters.html#thumbnail

View File

@@ -101,6 +101,8 @@ class IqdbClient
preview = media_file.preview!(Danbooru.config.small_image_width, Danbooru.config.small_image_width) preview = media_file.preview!(Danbooru.config.small_image_width, Danbooru.config.small_image_width)
file = HTTP::FormData::File.new(preview) file = HTTP::FormData::File.new(preview)
request(:post, "query", form: { file: file }, params: { limit: limit }) request(:post, "query", form: { file: file }, params: { limit: limit })
ensure
preview&.close
end end
# Add a post to IQDB. # Add a post to IQDB.

View File

@@ -7,6 +7,11 @@
class MediaFile::Image < MediaFile class MediaFile::Image < MediaFile
delegate :thumbnail_image, to: :image delegate :thumbnail_image, to: :image
def close
super
@preview_frame&.close unless @preview_frame == self
end
def dimensions def dimensions
image.size image.size
rescue Vips::Error rescue Vips::Error
@@ -107,7 +112,7 @@ class MediaFile::Image < MediaFile
resized_image = resized_image.flatten(background: 255) resized_image = resized_image.flatten(background: 255)
end end
output_file = Tempfile.new(["image-preview-#{md5}", ".#{format.to_s}"]) output_file = Danbooru::Tempfile.new(["danbooru-image-preview-#{md5}-", ".#{format.to_s}"])
case format.to_sym case format.to_sym
when :jpeg when :jpeg
# https://www.libvips.org/API/current/VipsForeignSave.html#vips-jpegsave # https://www.libvips.org/API/current/VipsForeignSave.html#vips-jpegsave
@@ -130,14 +135,6 @@ class MediaFile::Image < MediaFile
preview_frame.resize!(w, h, size: :force, **options) preview_frame.resize!(w, h, size: :force, **options)
end end
def preview_frame
if is_animated?
FFmpeg.new(self).smart_video_preview
else
self
end
end
def is_animated? def is_animated?
frame_count.to_i > 1 frame_count.to_i > 1
end end
@@ -166,6 +163,8 @@ class MediaFile::Image < MediaFile
false false
end end
private
# @return [Vips::Image] the Vips image object for the file # @return [Vips::Image] the Vips image object for the file
def image def image
Vips::Image.new_from_file(file.path, fail: false).autorot Vips::Image.new_from_file(file.path, fail: false).autorot
@@ -175,5 +174,15 @@ class MediaFile::Image < MediaFile
FFmpeg.new(self) FFmpeg.new(self)
end end
memoize :image, :video, :preview_frame, :dimensions, :error, :metadata, :is_corrupt?, :is_animated_gif?, :is_animated_png? def preview_frame
@preview_frame ||= begin
if is_animated?
FFmpeg.new(self).smart_video_preview
else
self
end
end
end
memoize :image, :video, :dimensions, :error, :metadata, :is_corrupt?, :is_animated_gif?, :is_animated_png?
end end

View File

@@ -17,8 +17,9 @@ class MediaFile::Ugoira < MediaFile
end end
def close def close
file.close super
preview_frame.close @preview_frame&.close
# XXX should clean up `convert` too
end end
def metadata def metadata
@@ -52,7 +53,7 @@ class MediaFile::Ugoira < MediaFile
raise RuntimeError, "can't convert ugoira to webm: no ugoira frame data was provided" unless frame_delays.present? raise RuntimeError, "can't convert ugoira to webm: no ugoira frame data was provided" unless frame_delays.present?
Danbooru::Archive.extract!(file) do |tmpdir, filenames| Danbooru::Archive.extract!(file) do |tmpdir, filenames|
output_file = Tempfile.new(["ugoira-conversion", ".webm"], binmode: true) output_file = Danbooru::Tempfile.new(["danbooru-ugoira-conversion-#{md5}-", ".webm"], binmode: true)
# Duplicate last frame to avoid it being displayed only for a very short amount of time. # Duplicate last frame to avoid it being displayed only for a very short amount of time.
last_file_name = File.basename(filenames.last) last_file_name = File.basename(filenames.last)
@@ -90,8 +91,8 @@ class MediaFile::Ugoira < MediaFile
private private
def preview_frame def preview_frame
FFmpeg.new(convert).smart_video_preview @preview_frame ||= FFmpeg.new(convert).smart_video_preview
end end
memoize :preview_frame, :dimensions, :convert, :metadata memoize :dimensions, :convert, :metadata
end end

View File

@@ -10,6 +10,11 @@ class MediaFile::Video < MediaFile
:audio_stream, :audio_streams, :silence_duration, :silence_percentage, :average_loudness, :audio_stream, :audio_streams, :silence_duration, :silence_percentage, :average_loudness,
:peak_loudness, :loudness_range, :error, to: :video :peak_loudness, :loudness_range, :error, to: :video
def close
super
@preview_frame&.close
end
def dimensions def dimensions
[video.width, video.height] [video.width, video.height]
end end
@@ -65,8 +70,8 @@ class MediaFile::Video < MediaFile
end end
def preview_frame def preview_frame
video.smart_video_preview @preview_frame ||= video.smart_video_preview
end end
memoize :video, :preview_frame, :dimensions, :metadata, :duration, :has_audio? memoize :video, :dimensions, :metadata, :duration, :has_audio?
end end

View File

@@ -53,6 +53,8 @@ class PostReplacementProcessor
rescue Exception => exception rescue Exception => exception
replacement.errors.add(:base, exception.message) replacement.errors.add(:base, exception.message)
raise ActiveRecord::Rollback raise ActiveRecord::Rollback
ensure
media_file&.close
end end
def rescale_notes(post) def rescale_notes(post)

View File

@@ -27,7 +27,7 @@ class StorageManager::Rclone < StorageManager
end end
def open(path) def open(path)
file = Tempfile.new(binmode: true) file = Danbooru::Tempfile.new(binmode: true)
rclone "copyto", key(path), file.path rclone "copyto", key(path), file.path
file file
end end

View File

@@ -68,6 +68,8 @@ class MediaAsset < ApplicationRecord
file = convert_file(original_file) file = convert_file(original_file)
storage_service.store(file, file_path) storage_service.store(file, file_path)
backup_storage_service.store(file, file_path) backup_storage_service.store(file, file_path)
ensure
file&.close
end end
def trash_file! def trash_file!

View File

@@ -112,6 +112,8 @@ class UploadMediaAsset < ApplicationRecord
update!(status: :active) update!(status: :active)
rescue Exception => e rescue Exception => e
update!(status: :failed, error: e.message) update!(status: :failed, error: e.message)
ensure
media_file&.close
end end
def update_upload_status def update_upload_status

View File

@@ -2,7 +2,7 @@ require 'test_helper'
class StorageManagerTest < ActiveSupport::TestCase class StorageManagerTest < ActiveSupport::TestCase
def tempfile(data) def tempfile(data)
file = Tempfile.new file = Danbooru::Tempfile.new
file.write(data) file.write(data)
file.flush file.flush
file file