Skip to content

Commit

Permalink
Make the library fork safe and drop the mutex
Browse files Browse the repository at this point in the history
When forking, file descriptors are inherited and their state shared.

In the context of MiniMime this means that the offset of the file
opened by RandomAccessDb is shared across processes, so the
`seek + read` combo is subject to inter-process race conditions.

Of course that file is lazily opened, so assuming most applications
don't query MiniMime before fork, it's not a big problem.

However when reforking post boot (e.g. https://github.com/Shopify/pitchfork)
this becomes an issue.

Additionally, even if the file descriptor isn't shared across processes,
the file position is still process global requiring a Mutex.

By using `pread` instead of `seek + read` we can both make the library
fork safe and get rid of the need to synchronize accesses.

This also happens to fix an outstanding JRuby issue.

Fix: #37
Fix: #38
  • Loading branch information
byroot authored and SamSaffron committed Aug 3, 2023
1 parent 4fd65d8 commit f67eef6
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 15 deletions.
19 changes: 6 additions & 13 deletions lib/mini_mime.rb
Expand Up @@ -50,8 +50,6 @@ def binary?
end

class Db
LOCK = Mutex.new

def self.lookup_by_filename(filename)
extension = File.extname(filename)
return if extension.empty?
Expand All @@ -60,18 +58,14 @@ def self.lookup_by_filename(filename)
end

def self.lookup_by_extension(extension)
LOCK.synchronize do
@db ||= new
@db.lookup_by_extension(extension) ||
@db.lookup_by_extension(extension.downcase)
end
@db ||= new
@db.lookup_by_extension(extension) ||
@db.lookup_by_extension(extension.downcase)
end

def self.lookup_by_content_type(content_type)
LOCK.synchronize do
@db ||= new
@db.lookup_by_content_type(content_type)
end
@db ||= new
@db.lookup_by_content_type(content_type)
end

class Cache
Expand Down Expand Up @@ -146,8 +140,7 @@ def lookup_uncached(val)
end

def resolve(row)
@file.seek(row * @row_length)
Info.new(@file.readline)
Info.new(@file.pread(@row_length, row * @row_length).force_encoding(Encoding::UTF_8))
end
end

Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/custom_content_type_mime.db
@@ -1,2 +1,2 @@
liquid application/x-liquid 8bit
liquid application/x-liquid 8bit
mp4 video/vnd.objectvideo quoted-printable
2 changes: 1 addition & 1 deletion test/fixtures/custom_ext_mime.db
@@ -1,2 +1,2 @@
lua application/x-lua 8bit
lua application/x-lua 8bit
m4v video/vnd.objectvideo quoted-printable

0 comments on commit f67eef6

Please sign in to comment.