Fix the deadlock on dealing with incoming email

Fixes #336

There was an occasional deadlock when two emails for the
same request came in near-simultaneously; two processes
would be started via script/mailin, each to deal with one
email which are both updating the same InfoRequest.

The error would look like:

   2013-04-07 09:19:03 BST [13398]: [2-1] DETAIL:  Process 13398 waits for ShareLock on transaction 36193647; blocked by process 13397.
   Process 13397 waits for ExclusiveLock on tuple (390,35) of relation 32918788 of database 32918687; blocked by process 13398.
   Process 13398: UPDATE "info_requests" SET "updated_at" = '2013-04-07 08:19:02.139515', "awaiting_description" = 't' WHERE "id" = 156200
   Process 13397: UPDATE "info_requests" SET "updated_at" = '2013-04-07 08:19:02.143624', "awaiting_description" = 't' WHERE "id" = 156200

This arose from the following section of code:

    ActiveRecord::Base.transaction do
        raw_email = RawEmail.new
        incoming_message.raw_email = raw_email
        incoming_message.info_request = self
        incoming_message.save!
        raw_email.data = raw_email_data
        raw_email.save!

        self.awaiting_description = true
        params = { :incoming_message_id => incoming_message.id }
        if !rejected_reason.empty?
            params[:rejected_reason] = rejected_reason.to_str
        end
        self.log_event("response", params)
        self.save!
    end

Matthew Somerville explained what was happening here in the
issue report; to repeat his explanation from the bug report,
both processes enter the transaction block and acquire a
ShareLock on self with:

   incoming_message.info_request = self
   incoming_message.save!

However, in order to update the self.awaiting_description field
of the InfoRequest, with:

   self.awaiting_description = true
   [...]
   self.save!

... the ShareLock needs to be upgraded to an ExclusiveLock,
but both will wait until the other's ShareLock is released,
which would only happen at the end of the transaction.

We can avoid this deadlock by using SELECT ... FOR UPDATE
for the row in info_requests.  In Rails 3.2.0 there is
ActiveRecord support for this (via with_lock and lock! on
a model instance) but so as not to require upgrading rails,
I'm just using raw SQL.

1 files changed, 11 insertions, 0 deletions

diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 237364f56..eaed25a61 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -474,6 +474,17 @@ public
         incoming_message = IncomingMessage.new
 
         ActiveRecord::Base.transaction do
+
+            # To avoid a deadlock when simultaneously dealing with two
+            # incoming emails that refer to the same InfoRequest, we
+            # lock the row for update.  In Rails 3.2.0 and later this
+            # can be done with info_request.with_lock or
+            # info_request.lock!, but upgrading to that version of
+            # Rails creates many other problems at the moment.  In the
+            # interim, just use raw SQL to do the SELECT ... FOR UPDATE
+            raw_sql = "SELECT * FROM info_requests WHERE id = #{self.id} LIMIT 1 FOR UPDATE"
+            ActiveRecord::Base.connection.execute(raw_sql)
+
             raw_email = RawEmail.new
             incoming_message.raw_email = raw_email
             incoming_message.info_request = self