diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4372e9f139636..f5da038de649e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -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. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 68af021a06122..8735689b4eea6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -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 diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index df1a2656b5db0..427ebf09b2f8b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -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! diff --git a/activerecord/lib/active_record/connection_adapters/pool_config.rb b/activerecord/lib/active_record/connection_adapters/pool_config.rb index 0010a48acd225..1db570fb32135 100644 --- a/activerecord/lib/active_record/connection_adapters/pool_config.rb +++ b/activerecord/lib/active_record/connection_adapters/pool_config.rb @@ -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 @@ -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 @@ -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! } diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 888741ba262c4..d73b0ee40155e 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -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.