classification
Title: unicode_concatenate() optimization is not signal-safe (not atomic)
Type: Stage:
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Ted Meyer, arigo, llllllllll
Priority: normal Keywords: patch

Created on 2016-12-28 20:38 by Ted Meyer, last changed 2017-01-05 07:06 by arigo.

Files
File name Uploaded Description Edit
patch2.diff arigo, 2017-01-05 07:04 review
patch1.diff arigo, 2017-01-05 07:06 review
Messages (4)
msg284196 - (view) Author: Ted Meyer (Ted Meyer) Date: 2016-12-28 20:38
Using this simple snippit of code:
import signal



import signal

def sig_hdlr(signum, frame):
    raise ValueError()

def faulty():
    local_var = ""
    signal.signal(signal.SIGALRM, sig_hdlr)
    signal.alarm(1)
    try:
        while True:
            local_var += "!"
    except ValueError:
        print (local_val)

faulty()



I can reliably get a crash:
tmathmeyer@tmathmeyer-linuxstation:~$ python --version
Python 2.7.6
tmathmeyer@tmathmeyer-linuxstation:~$ python pybug.py 
Traceback (most recent call last):
  File "pybug.py", line 16, in <module>
    faulty()
  File "pybug.py", line 14, in faulty
    print local_val
NameError: global name 'local_val' is not defined

tmathmeyer@tmathmeyer-linuxstation:~$ python3.4 pybug.py 
Traceback (most recent call last):
  File "pybug.py", line 12, in faulty
    local_var += "!"
  File "pybug.py", line 4, in sig_hdlr
    raise ValueError()
ValueError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "pybug.py", line 16, in <module>
    faulty()
  File "pybug.py", line 14, in faulty
    print (local_val)
NameError: name 'local_val' is not defined


I can repro this on 2.7.6 and 3.4, but do not have other versions to test on.
msg284197 - (view) Author: Ted Meyer (Ted Meyer) Date: 2016-12-28 20:46
Sorry everyone, it appears I made a mistake pasting the code in here.
There was a type where I was trying to print local_val instead of local_var. the code should be this:

import signal

def sig_hdlr(signum, frame):
    raise ValueError()

def faulty():
    local_var = ""
    signal.signal(signal.SIGALRM, sig_hdlr)
    signal.alarm(1)
    try:
        while True:
            local_var += "!"
    except ValueError:
        print (local_var)

faulty()


and the crash should be this:
Traceback (most recent call last):
  File "pybug.py", line 12, in faulty
    local_var += "!"
  File "pybug.py", line 4, in sig_hdlr
    raise ValueError()
ValueError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "pybug.py", line 16, in <module>
    faulty()
  File "pybug.py", line 14, in faulty
    print (local_var)
UnboundLocalError: local variable 'local_var' referenced before assignment
msg284198 - (view) Author: Joe Jevnik (llllllllll) * Date: 2016-12-28 20:48
The issue appears to be in ceval.c:unicode_concatenate (or the py2 equivalent)

The code sets the local variable on the lhs to NULL before doing a potentially inplace append to the string. This means that if a signal is raised during the concat, we never hit the STORE_FAST instruction following the INPLACE_ADD so the local or cell still holds NULL, triggering an error.

I am not really sure what can be done to prevent the failure while still allowing the optimization.
msg284710 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2017-01-05 07:03
The signal handler is called between the INPLACE_ADD and the following STORE_FAST opcode, never from string_concatenate() itself.  A fix would be to make sure signal handlers are not called between these two opcodes.  See the minimal, proof-of-concept patch #1.  A possibly better fix, which at least should match the SETUP_FINALLY handling in the ticker handler, is attached as patch #2.  (Patches against 2.7.13)
History
Date User Action Args
2017-01-05 07:06:58arigosetfiles: + patch1.diff
2017-01-05 07:05:32arigosetfiles: - patch1.diff
2017-01-05 07:04:31arigosetfiles: + patch2.diff
2017-01-05 07:03:36arigosetfiles: + patch1.diff
keywords: + patch
messages: + msg284710
2017-01-05 04:44:25rhettingersetnosy: + arigo
2017-01-03 14:19:08vstinnersettitle: Signal Handlers reliably cause UnboundLocalErrors -> unicode_concatenate() optimization is not signal-safe (not atomic)
versions: + Python 3.5, Python 3.6, Python 3.7, - Python 3.3, Python 3.4
2016-12-28 20:49:00llllllllllsetnosy: + llllllllll
messages: + msg284198

components: + Interpreter Core
type: crash ->
2016-12-28 20:46:49Ted Meyersetmessages: + msg284197
2016-12-28 20:41:59Ted Meyersettype: crash
2016-12-28 20:38:35Ted Meyercreate