Skip to content

Commit

Permalink
SQLite3Adapter: Ensure fork-safety
Browse files Browse the repository at this point in the history
Sqlite itself is not fork-safe, as documented in
https://www.sqlite.org/howtocorrupt.html section 2.6. Any databases
that are inherited from the parent process are unsafe to properly
close and may impact either the parent or the child and potentially
lead to database corruption.

In this commit, a pre-fork callback `prepare_to_fork!` is introduced
for the connection pools and adapters to take any necessary actions
before forking. The callback is only implemented by the SQLite3Adapter
which now invokes `disconnect!` to close all open prepared statements
and database connections.

As a side effect, the parent process will end up having to re-open
database connections if it continues to do work, which may be a small
performance overhead for some use cases, but is necessary in order to
prevent database corruption.

See rails/solid_queue#324 for examples of
the type of corruption that can occur.
  • Loading branch information
flavorjones committed Sep 14, 2024
1 parent afd7578 commit eadbfb7
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 0 deletions.
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
* SQLite3Adapter: Ensure fork-safety.

Previously, forking a Rails process with open SQLite3 database connections could result in
database file corruption under certain circumstances.

*Mike Dalessio*

* Add support for PostgreSQL `IF NOT EXISTS` via the `:if_not_exists` option
on the `add_enum_value` method.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,16 @@ def disconnect(raise_on_acquisition_timeout = true)
end
end

# Notify connections that a fork is imminent, to allow them to take necessary measures to
# ensure fork-safety.
#
# See AbstractAdapter#prepare_to_fork!
def prepare_to_fork! # :nodoc:
@connections.each do |conn|
conn.prepare_to_fork!
end
end

# Disconnects all connections in the pool, and clears the pool.
#
# The pool first tries to gain ownership of all connections. If unable to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,13 @@ def reconnect!(restore_transactions: false)
end
end

# Prepare the connection for an imminent fork. Some database adapters are not fork-safe
# (specifically sqlite3), and so this gives the adapter an opportunity to close or finalize
# any objects that the child process should not inherit.
def prepare_to_fork!
# This should be overridden by concrete adapters.
end

# Disconnects from the database if already connected. Otherwise, this
# method does nothing.
def disconnect!
Expand Down
13 changes: 13 additions & 0 deletions activerecord/lib/active_record/connection_adapters/pool_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ def schema_reflection
private_constant :INSTANCES

class << self
def prepare_to_fork!
INSTANCES.each_key(&:prepare_to_fork!)
end

def discard_pools!
INSTANCES.each_key(&:discard_pool!)
end
Expand Down Expand Up @@ -66,6 +70,14 @@ def pool
@pool || synchronize { @pool ||= ConnectionAdapters::ConnectionPool.new(self) }
end

def prepare_to_fork!
synchronize do
return unless @pool

@pool.prepare_to_fork!
end
end

def discard_pool!
return unless @pool

Expand All @@ -80,4 +92,5 @@ def discard_pool!
end
end

ActiveSupport::ForkTracker.before_fork { ActiveRecord::ConnectionAdapters::PoolConfig.prepare_to_fork! }
ActiveSupport::ForkTracker.after_fork { ActiveRecord::ConnectionAdapters::PoolConfig.discard_pools! }
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,19 @@ def initialize_type_map(m)
TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) }
EXTENDED_TYPE_MAPS = Concurrent::Map.new

def prepare_to_fork!
# It's not safe to fork with an open sqlite connection, because when the object is GCed in
# the child process, it can interfere with existing or subsequent connections to the
# database in the child process, which can lead to corruption.
#
# Note that closing the database will raise a BusyException unless all related statements
# are also closed, which the AbstractAdapter's disconnect! method handles for us.
#
# More on sqlite fork-safety: https://www.sqlite.org/howtocorrupt.html section 2.6
# More on Rails db corruption: https://github.com/rails/solid_queue/issues/324
disconnect!
end

private
# See https://www.sqlite.org/limits.html,
# the default value is 999 when not configured.
Expand Down

0 comments on commit eadbfb7

Please sign in to comment.