Opened 4 years ago

Closed 4 years ago

#31315 closed Bug (wontfix)

Closing all connections in function tagged with @transaction_atomic or transaction atomic block breaks atomicity

Reported by: David Eling Owned by: nobody
Component: Database layer (models, ORM) Version: 2.1
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Bug might be related to https://code.djangoproject.com/ticket/21239

If you use the transaction atomic tag on a function ala

@transaction.atomic
def post(self, request):
   Machine.objects.create(name="a")
   connections.close_all()
   connection.connect()
   list(Machine.objects.all())
   Machine.objects.create(name="b")
       
   return JsonResponse({})

b will exist but a will not.

Now the code above is way simplified compared to what actually caused the bug in my codebase but it replicates the issue (although it seems very arbitrary in its current form) The codebase in question is very complex and involves multiple databases on different machines, this is my best attempt at boiling it down to what bits caused what. So it calls connections.close_all() and then reopens the connection to the current db then does a query then later down the line it does a create.

The issue will occur if you do this instead

def post(self, request):
   with transaction.atomic():
      Machine.objects.create(name="a")
      connections.close_all()
      connection.connect()
      list(Machine.objects.all())
      Machine.objects.create(name="b")
       
    return JsonResponse({})

During further testing it looks like swapping connections.close_all() with connection.close() still causes the issue to occur, so maybe the fix in the issue linked above did not fix the issue all the way.

It's possible this bug is only happening due to the code exploiting some loophole or something but I just thought I would put this here just in case it might be a real problem that needs to be fixed. Within our code base we had to isolate the offending bit and do it before entering the atomic block.

This is my first time reporting a django bug so I apologize for any issues.

Change History (1)

comment:1 by Carlton Gibson, 4 years ago

Resolution: wontfix
Status: newclosed

Hi David. Thanks for the report. Interesting... :)

I think this is expected behaviour. The docs go as far as:

connection and cursor mostly implement the standard Python DB-API described in PEP 249

That defines the behaviour for `close()`:

Note that closing a connection without committing the changes first will cause an implicit rollback to be performed.

Looking at the code, we have exactly that:

            if self.in_atomic_block:
                self.closed_in_transaction = True
                self.needs_rollback = True

Within our code base we had to isolate the offending bit and do it before entering the atomic block.

I think that's the right approach. My first guess reading this would be that closing the connection inside a transaction would be at best undefined. What do I expect to happen there? — I could see a few answers to that. That it's rollback is no real surprise.

I hope that makes sense?

Kind Regards,

Carlton

Note: See TracTickets for help on using tickets.
Back to Top