Rewrite logging hack of the threading module

[vstinner]

BPO	13550
Nosy	@tim-one, @jcea, @ncoghlan, @pitrou, @vstinner, @merwok
Files	threading_note.patch threading_note_global.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2012-03-03.00:35:31.678>
created_at = <Date 2011-12-07.22:40:25.967>
labels = ['library']
title = 'Rewrite logging hack of the threading module'
updated_at = <Date 2012-03-03.00:35:31.676>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2012-03-03.00:35:31.676>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2012-03-03.00:35:31.678>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2011-12-07.22:40:25.967>
creator = 'vstinner'
dependencies = []
files = ['23869', '23871']
hgrepos = []
issue_num = 13550
keywords = ['patch']
message_count = 10.0
messages = ['149001', '149003', '149084', '149166', '149823', '150803', '150805', '152820', '152824', '154815']
nosy_count = 8.0
nosy_names = ['tim.peters', 'jcea', 'ncoghlan', 'pitrou', 'vstinner', 'eric.araujo', 'neologix', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue13550'
versions = ['Python 3.3']

[vstinner]

The threading module uses an hack to log actions to help debugging. The log depends on 3 flags: __debug__, threading._VERBOSE and a verbose attribute (each threading class has such attribute). By default, _note() is always called but does nothing: it checks the verbose attribute and return if the flag is False.

Attached threading_note.patch only calls _note() if verbose is True to avoid a Python function call (which is slow in CPython). It avoids also a _Verbose parent class and pass the verbose flag to subobjects (e.g. Semaphore sets verbose argument of its Condition object).

I don't think that it is really useful to be able to enable/disable logs on only one threading object, so it is maybe simpler to have only one global flag (instead of 3 flags): threading._VERBOSE. Attached threading_note_global.patch replaces _note() method by a function, remove _Verbose class and the _verbose attribute, and only call _note() if _VERBOSE is True.

_VERBOSE and verbose arguments are undocumented and not tested, so I hope nobody relies on their API ;-)

[vstinner]

(threading_note_global.patch was completly wrong, here is the fixed version)

[pitrou]

Tim, do you happen to know what the goal was with the threading._VERBOSE hack and the undocumented _Verbose class?

[merwok]

On bpo-13550 I asked Guido about the Thing/_Thing function/class indirection and use of _Verbose; the reply:

IIRC:

The design started out this way because it predates new-style classes. When this was put in one
couldn't subclass extension types, and there were plans/hopes to replace some of the lock types
with platform-specific built-in versions on some platforms.

Since it is now possible to write subclassable extension types the Thing/_Thing design is no
longer needed.

I'm not sure about the _Verbose hacks, if it is deemed not useful I'm fine with letting it go.

[neologix]

I'm personally +1 on removing the verbose thing altogether:

• it's ugly
• I doubt it's really useful (I mean, printing to stderr - which is often line buffered or unbuffered - upon every action will probably change the timing)
• it also brings some problems on its own, e.g. bpo-4188

So unless there's a good reason behind it, I think we could let it go.

[neologix]

Alright, Nick agreed on python-dev to remove the logging hack.

[merwok]

haypo’s threading_note_global looks good to me. The only thing I’m not sure about is the signature change from X(verbose, *args, **kwargs) to X(*args, **kwargs): is it okay?

(BTW you probably want to delete the obsolete reference to ihooks in a comment before committing.)

[vstinner]

Alright, Nick agreed on python-dev to remove the logging hack.

You mean removing complelty debug logging from the threading module? Or just to simplify the code to decide if we should log or not?

[ncoghlan]

I believe Charles-François was referring to this message:
http://mail.python.org/pipermail/python-dev/2012-January/115372.html

We shouldn't be encumbering threading *all the time* with stuff that "might be useful sometimes". Adding selective output to help debug problems can be handled with Thread subclasses, or even temporarily running with a modified threading.py (for people hacking on the stdlib itself).

[python-dev]

New changeset 8ec51b2e57c2 by Victor Stinner in branch 'default':
Close bpo-13550: Remove the debug machinery from the threading module: remove
http://hg.python.org/cpython/rev/8ec51b2e57c2