Opened 6 years ago

Closed 12 days ago

Last modified 25 hours ago

#16682 closed Bug (fixed)

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 5 months ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 6 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 5 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 5 months ago by Tim Graham

Attachment: 16682-atomic.diff added

comment:9 Changed 5 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 5 months 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 3 months ago by Tim Graham

Cc: felixxm added

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

comment:12 Changed 3 months 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 3 months 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 3 months ago by Tim Graham <timograham@…>

In d391b3a:

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

comment:15 Changed 3 months 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 3 months 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.

comment:17 Changed 3 months ago by felixxm

Oracle DB is in the same version in both cases (12.1.0.2.0). I also checked DB params and there is nothing unusual in them. I don't know where is the source of the problem.

comment:18 Changed 6 weeks ago by Josh Smeaton

I just hit this too.

Dev Days VM: 2016-06-02_09_53_26 64 bit.
Tests OS: OSX 10.12.2
Python 3.5.2
cx_Oracle: 5.2.1

comment:19 Changed 12 days ago by Tim Graham

Resolution: fixed
Status: newclosed

I guess there's nothing to be done. Hopefully it's solved in a future version of the Oracle VM.

comment:20 Changed 25 hours ago by Tim Graham

There's another problem with this test. When running ./tests/runtests.py transactions --parallel=1, this test halts execution so that only 26 tests are run. After commenting this test out, the same command runs 72 tests. Maybe it's better to remove this test for now.

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