Opened 5 years ago

Last modified 7 days ago

#16682 new Bug

KeyboardInterrupt not handled properly in transaction aborting

Reported by: Malcolm Tredinnick Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords: 1.11
Cc: felixxm Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Ticket #6928 was reopened to report this, so I (Malcolm) am moving it to a new ticket, since it's a separate issue. Following text is from comment:7 on that ticket:

---

After [14288] transaction.commit_on_success does not handle KeyboardInterrupt.

Problem in django.db.transaction.Transaction.__exit__ and default exiting function.

    def __exit__(self, exc_type, exc_value, traceback):
        self.exiting(exc_value, self.using)

exc_value is None after KeyboardInterrupt has been throwed, only exc_type and traceback has value.
But if raise KeyboardInterrupt manualy from code it's work ok.

Possible solution: use exc_type and exc_value in exiting function

I use python 2.6.6

Attachments (2)

16682-test.diff (1.1 KB) - added by Claude Paroz 5 years ago.
Failing test
16682-atomic.diff (1.0 KB) - added by Tim Graham 2 months ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 5 years ago by Aymeric Augustin

Resolution: needsinfo
Status: newclosed

I'm sorry, but this report doesn't contain enough information to reproduce the problem. How can you get a KeyboardInterrupt within transaction.commit_on_success?

I wrote this test code:

# models.py

from django.db import models

class FooModel(models.Model):
    foo = models.CharField(max_length=42)

# views.py

import time
from django.db import transaction
from .models import FooModel

@transaction.commit_on_success
def interrupt_me(request):
    FooModel.objects.create(foo='foo')
    print "Hit Ctrl-C now!"
    time.sleep(5)
    print "Too late, and I didn't bother to return a HttpResponse"

and hooked the view in the URLconf.

Then I ran: ./manage.py runserver --traceback

I open the URL in a browser, and during the sleep, I hit Ctrl-C in the console. runserver exits cleanly, with or without transaction.commit_on_success.

Please provide a test case or instructions to reproduce your problem and re-open the ticket.

comment:4 Changed 5 years ago by Claude Paroz

Resolution: needsinfo
Status: closedreopened
Triage Stage: UnreviewedAccepted

I've been able to reproduce it with the following code (you can insert it in regressiontests/transactions_regress/tests.py):

    def test_rollback_on_keyboardinterrupt(self):
        import time
        try:
            with transaction.commit_on_success():
                Mod.objects.create(fld=17624)
                print "Hit Ctrl-C now!"
                time.sleep(5)
                print "Too late, and I didn't bother to return a HttpResponse"
        except KeyboardInterrupt:
            pass
        self.assertEqual(Mod.objects.all().count(), 0)

Unfortunately, I didn't manage to simulate a Ctrl-C programmatically, so as to be able to include a real test. Ideas welcome.

comment:5 Changed 5 years ago by Claude Paroz

#17624 is a duplicate with a patch

Changed 5 years ago by Claude Paroz

Attachment: 16682-test.diff added

Failing test

comment:6 Changed 5 years ago by Claude Paroz

I eventually found a trick to generate the KeyboardInterrupt to create a failing test. Don't ask me why the os.kill has to be called twice...

comment:7 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:8 Changed 2 months ago by Tim Graham

transaction.atomic() doesn't seem to suffer from this issue. Is there any value in adding the test (an updated one is attached)?

Changed 2 months ago by Tim Graham

Attachment: 16682-atomic.diff added

comment:9 Changed 2 months ago by Tim Graham

Has patch: set
Resolution: fixed
Status: newclosed
Triage Stage: AcceptedReady for checkin

I created a PR and Aymeric indicated that adding the test is useful.

comment:10 Changed 7 weeks ago by Tim Graham

Has patch: unset
Keywords: 1.11 added
Resolution: fixed
Status: closednew
Triage Stage: Ready for checkinAccepted

Reopening as the test hangs on the last line when I run it on Oracle (using the Oracle DB Developer VM). I haven't seen any problems with it on Jenkins. Can anyone else reproduce that and/or have an idea about the cause?

comment:11 Changed 11 days ago by Tim Graham

Cc: felixxm added

Mariusz, are you able to reproduce the hang on Oracle? Any ideas?

comment:12 Changed 10 days ago by felixxm

I wasn't able to reproduce the hang on Oracle 11g/12c. I will try later with Oracle DB Developer VM. Small suggestion is to use signal.SIGINT instead of signal number.

--- a/tests/transactions/tests.py
+++ b/tests/transactions/tests.py
@@ -1,6 +1,7 @@
 from __future__ import unicode_literals
 
 import os
+import signal
 import sys
 import threading
 import time
@@ -225,8 +226,8 @@ class AtomicTests(TransactionTestCase):
             with transaction.atomic():
                 Reporter.objects.create(first_name='Tintin')
                 # Send SIGINT (simulate Ctrl-C). One call isn't enough.
-                os.kill(os.getpid(), 2)
-                os.kill(os.getpid(), 2)
+                os.kill(os.getpid(), signal.SIGINT)
+                os.kill(os.getpid(), signal.SIGINT)
         except KeyboardInterrupt:
             pass
         self.assertEqual(Reporter.objects.all().count(), 0)

comment:13 Changed 8 days ago by felixxm

I confirmed that test_rollback_on_keyboardinterrupt hangs on Oracle from Oracle DB Developer VM. I will try to figure it out in the next days.

comment:14 Changed 8 days ago by Tim Graham <timograham@…>

In d391b3a:

Refs #16682 -- Replaced signal number with equivalent signal.SIGINT.

comment:15 Changed 7 days ago by felixxm

IMO it is not a Django issue. Test hangs on ​rollback, hence it is probably Oracle or cx_Oracle bug.

comment:16 Changed 7 days ago by Tim Graham

Thanks for investigating. Since this is the only documented way we have of running the tests on Oracle for development, is there a chance we could skip the test when running with the Developer Days VM? Maybe the Oracle version is different from the version on Jenkins, for example.

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