Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_dummy_threading vs -O #40592

Closed
tim-one opened this issue Jul 18, 2004 · 9 comments
Closed

test_dummy_threading vs -O #40592

tim-one opened this issue Jul 18, 2004 · 9 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@tim-one
Copy link
Member

tim-one commented Jul 18, 2004

BPO 993394
Nosy @tim-one, @brettcannon

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 = 'https://github.com/brettcannon'
closed_at = <Date 2004-07-21.02:56:50.000>
created_at = <Date 2004-07-18.18:31:29.000>
labels = ['interpreter-core']
title = 'test_dummy_threading vs -O'
updated_at = <Date 2004-07-21.02:56:50.000>
user = 'https://github.com/tim-one'

bugs.python.org fields:

activity = <Date 2004-07-21.02:56:50.000>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2004-07-18.18:31:29.000>
creator = 'tim.peters'
dependencies = []
files = []
hgrepos = []
issue_num = 993394
keywords = []
message_count = 9.0
messages = ['21670', '21671', '21672', '21673', '21674', '21675', '21676', '21677', '21678']
nosy_count = 2.0
nosy_names = ['tim.peters', 'brett.cannon']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue993394'
versions = ['Python 2.4']

@tim-one
Copy link
Member Author

tim-one commented Jul 18, 2004

In Python 2.4, running test_dummy_threading with -O
leads to an exception after the test is finished, in
threading's exit func. Didn't happen under 2.3. Sorry,
no idea why, and as soon as I saw all the fiddling with
sys.modules I ran screaming in terror <wink>. Here on
WinXP:

C:\Python23>python lib/test/regrtest.py
test_dummy_threading # OK
test_dummy_threading
1 test OK.

C:\Python23>python -O lib/test/regrtest.py
test_dummy_threading # OK
test_dummy_threading
1 test OK.

C:\Python23>cd \python24

C:\Python24>python lib/test/regrtest.py
test_dummy_threading # OK
test_dummy_threading
1 test OK.

C:\Python24>python -O lib/test/regrtest.py 
test_dummy_threading # not OK
test_dummy_threading
1 test OK.
Error in sys.exitfunc:
Traceback (most recent call last):
  File "C:\Python24\lib\atexit.py", line 20, in 
_run_exitfuncs
    func(*targs, **kargs)
  File "C:\Python24\lib\threading.py", line 607, in 
__exitfunc
    self._Thread__delete()
  File "C:\Python24\lib\threading.py", line 497, in __delete
    del _active[_get_ident()]
KeyError: -1

C:\Python24>

@tim-one tim-one closed this as completed Jul 18, 2004
@tim-one tim-one added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jul 18, 2004
@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

This is just turning into the month of my regression tests breaking, or at
least my regression tests that decide to play dangeously with
sys.modules. =)

I will take a look and see if I can figure this one out this week.

@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

OK, on the way to figuring this out, I think.

Line 503 of Lib/threading.py has the statement assert self is not currentThread(). Now currentThread() creates an instance of
_DummyThread for the current thread and inserts it into
threading._active with the key of threading._get_ident(), which is -1 for
all dummy_threading Threads.

Problem is that this doesn't happen under -O so threading._active doesn't
get an entry for -1 like the exit func expects there to be and thus leads
to an error. You can uncomment line 645 to see the difference when the
test is run.

As for it being a new error in 2.4, my CVS checkout for 2.3 has this error
as well. Even if I back Lib/threading back to version 1.40 (before I made
changes for printing the traceback from shutdown errors) and
dummy_threading before Jim's threading local stuff (which puts it to the
initial checkin) I still get the error. So from what I can tell this is old.

Now, it isn't from the tests, that much I am sure. If you change the
imported module for _threading to 'threading' the tests pass either way.
For some reason the initial thread created from the _MainThread()
creation in 'threading' disappears before the end of execution for the
test. Not sure why. Might have something to do with
dummy_thread.get_ident() always returning -1. Going to keep looking
into it. If it is from dummy_thread.get_ident() returning -1 then it might
be a slight pain to try to fix. Might have to come up with some modified
dict for _active in dummy_threading for the code to use that just
consistently has a -1 or somehow communicates with
dummy_thread.get_ident() to keep everything in sync.

@tim-one
Copy link
Member Author

tim-one commented Jul 19, 2004

Logged In: YES
user_id=31435

Then I think you'll find the answer if you do

cvs diff -r r234 -r release23-maint threading.py

I was using the released 2.3.4 Python. But on the current
2.3 maint branch, you removed two lines like this:

        currentThread() # for side-effect

These *were* unconditionally executed in 2.3.4, but don't
exist on the current 2.3 branch, or on HEAD.

I expect that explains why this didn't fail in 2.3.4. I don't
know that reverting those lines is a correct (or even the
best) solution, though. Perhaps doing currentThread() once,
from the main thread, at the start of threading's shutdown
dance, would be enough.

@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

Can't add those lines back in without breaking the fixes to allow
tracebacks triggered during interpreter shutdown from failing. And I
don't think doing it at the beginning will solve it since it will still be
overwritten by subsequent threads in threading._active anyway. Could
be fixed if the call is made in the method registered with atexit.

Plan on spending tonight on this one. Hopefully I will come up with
something. =)

@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

OK, problem goes away if you add a currentThread() call in __exitfunc()
(line 607). This gives the desired side-effect all while being safe from
causing shutdown problems since that function is only called by atexit.
And I am about to run the whole test suite to make sure it doesn't cause
evil to appear somewhere else. =)

Does that solution work for you, Tim?

Otherwise I am afraid I am going to have to come up with a custom dict
for threading._active that dummy_threading injects into its copy that
tries to handle this properly.

@tim-one
Copy link
Member Author

tim-one commented Jul 20, 2004

Logged In: YES
user_id=31435

I don't think that will hurt anything. "The real problem" is of
course that len(_active) should be the number of threads,
and get_ident() should return a different int for each thread.
That seems painful to do if there aren't really different
threads.

An alternative would be to suppress KeyError in __delete().

Another alternative would be to suppress KeyError in __delete
() iff dummy_threading is in use.

@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

I like your last suggestion: doesn't get into side-effect dependence and
keeps any possible error detection for when threading isn't using
dummy_thread.

I'll go ahead and commit that fix.

@brettcannon
Copy link
Member

Logged In: YES
user_id=357491

OK, fixed in Lib/threading.py for rev. 1.44 in HEAD and rev. 1.38.6.3 for
release23-maint.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)
Projects
None yet
Development

No branches or pull requests

2 participants