[issue38076] Make struct module PEP-384 compatible

[Petr Viktorin]

Change by Petr Viktorin :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[miss-islington]

miss-islington  added the comment:


New changeset 4590f72259ecbcea66e12a28af14d867255d2e66 by Eddie Elizondo in 
branch 'master':
bpo-38076 Clear the interpreter state only after clearing module globals 
(GH-18039)
https://github.com/python/cpython/commit/4590f72259ecbcea66e12a28af14d867255d2e66


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Eric Snow]

Eric Snow  added the comment:

> there's still probably some underlying issue in multiprocessing.

Whoa, I've never heard that before!  

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

One more data point: Backporting this change to Python 3.6 (I just happened to 
have it applied there already, so I haven't tried it on 3.7 or 3.8) has no 
crash and no hangs in multiprocessing on Linux.  So something definitely 
changed in multiproessing which is causing the hang on shutdown, and forces us 
into this code path where we crash as well.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

With either fix, or with both, on Linux I still see this hang at shutdown.  
Victor mentioned the fact that he had to hit Ctrl-C on Linux to see this, and I 
have to do the same thing. Then with the fixes in place the original test case 
still hangs on shutdown.  

On Python 3.7 (I don't readily have 3.8 available) at least this just runs and 
completes with no ctrl-C and no crashes.  So while either of the fixes may be 
good to prevent the crashes, there's still probably some underlying issue in 
multiprocessing.  I haven't tested on Mac OS/X.

It looks like the clearing was originally introduced here: 
https://bugs.python.org/issue10241  Interestingly there was a similar issue w/ 
_tkinter, which also used PyType_FromSpec (although it sounds like it was just 
a ref count issue on the type).  Unfortunately there's no associated test cases 
added to verify the behavior.  Antoine and Neil are both now on the PR which 
removes the collection behavior so hopefully they can chime in on the safety of 
that fix.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Eddie Elizondo]

Eddie Elizondo  added the comment:

> I'm concerned by release blocker because 3.9.0a3 version is supposed to be 
> released soon, and usually release blocker do block a release :-)

Ah! That makes sense!

In any case, feel free to ping me if you need help on my side to get this PR 
through (or to remove the release blocker).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[STINNER Victor]

STINNER Victor  added the comment:

> That should close this issue, no need to work around the bug priority.

I'm concerned by release blocker because 3.9.0a3 version is supposed to be 
released soon, and usually release blocker do block a release :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Eddie Elizondo]

Eddie Elizondo  added the comment:

The PR that I sent out already fixes the issue. @vstinner, @pablogsal, please 
take a look again https://github.com/python/cpython/pull/18039

That should close this issue, no need to work around the bug priority.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[STINNER Victor]

STINNER Victor  added the comment:

> The error can still happened in other modules and under similar conditions, 
> no?

The question was if the next 3.9 *alpha* release must be blocked by this issue. 
I don't think so. I reduce the priority to normal (not set). If someone 
disagrees, feel free to raise it again to release blocker ;-)

--
priority: release blocker -> 

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> Is it enough to reduce the issue priority from release blocker to normal?

The error can still happened in other modules and under similar conditions, no?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[STINNER Victor]

STINNER Victor  added the comment:

> https://github.com/python/cpython/pull/18038 is a partial fix for this

Is it enough to reduce the issue priority from release blocker to normal?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Eddie Elizondo]

Eddie Elizondo  added the comment:

Hey all, I've got a fix for this bug and the CI is green: 
https://github.com/python/cpython/pull/18039

TL;DR: The module state have to be cleared at a later time. I explain in detail 
in the PR.

Also, I didn't add a new test since there was a test that was already checking 
for module states in `io`. I added a short comment on it in the PR as well. 
Otherwise, lmk and I can create a new test for it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Eddie Elizondo]

Change by Eddie Elizondo :


--
pull_requests: +17438
pull_request: https://github.com/python/cpython/pull/18039

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

https://github.com/python/cpython/pull/18038 is a partial fix for this.  I 
think it fixes the crash at shutdown, although I'm still seeing a hang on 
master on Linux which is different then earlier versions of Python.  I seem to 
have a really bogus stack trace when I attach to it so I'm not quite certain 
what's going on there.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Change by Dino Viehland :


--
pull_requests: +17437
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/18038

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

And here's a variation which doesn't involve any instances from the module:

import _struct

class C:
def __init__(self):
self.pack = _struct.pack
def __del__(self):
self.pack('I', -42)

_struct.x = C()

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

This is a relatively simple repro of the underlying problem:

import _struct

s = _struct.Struct('i')

class C:
def __del__(self):
s.pack(42, 100)

_struct.x = C()

It's a little bit different in that it is actually causing the module to 
attempt to throw an exception instead of do a type check.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Dino Viehland  added the comment:

It seems problematic that_PyInterpreterState_ClearModules runs before all 
instances from a module have been cleared.  If PyState_FindModule is no longer 
able to return module state then there's no way for a module to reliably work 
at shutdown other than having all instances hold onto the module (or module 
state) directly from all of their insatances.  Certainly that would mimic more 
closely what happens w/ pure Python instances and modules - the type will hold 
onto the functions which will hold onto the module global state.

--
nosy: +dino.viehland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Change by Dino Viehland :


--
nosy: +eelizondo -dino.viehland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[STINNER Victor]

STINNER Victor  added the comment:

I can reproduce the crash on Linux. I interrupt the problem with CTRL+c (why is 
it blocked? I don't know). Then it does crash.

First, _PyImport_Cleanup() does clear all modules including _struct.

Then, _PyImport_Cleanup() calls gc.collect() which finalize_garbage() which 
calls A.__del__().

Problem: at this point, the _struct became unusable.

--

Thread 1 "python" received signal SIGSEGV, Segmentation fault.
0x00473f30 in PyModule_GetState (m=0x0) at Objects/moduleobject.c:565
565 if (!PyModule_Check(m)) {
(gdb) py-bt
Traceback (most recent call first):
  
  File "/home/vstinner/python/master/Lib/multiprocessing/connection.py", line 
400, in _send_bytes
header = struct.pack("!i", n)
  File "/home/vstinner/python/master/Lib/multiprocessing/connection.py", line 
200, in send_bytes
self._send_bytes(m[offset:offset + size])
  File "/home/vstinner/python/master/Lib/multiprocessing/queues.py", line 368, 
in put
self._writer.send_bytes(obj)
  File "/home/vstinner/python/master/Lib/multiprocessing/pool.py", line 649, in 
close
self._change_notifier.put(None)
  File "/home/vstinner/python/master/x.py", line 7, in __del__
self.pool.close()
  Garbage-collecting

In debug mode, the crash occurs in s_pack() at:

assert(PyStruct_Check(self));

--

#define _structmodulestate(o) ((_structmodulestate *)PyModule_GetState(o))

static struct PyModuleDef _structmodule;

#define _structmodulestate_global 
_structmodulestate(PyState_FindModule(&_structmodule))

#define PyStruct_Check(op) PyObject_TypeCheck(op, (PyTypeObject 
*)_structmodulestate_global->PyStructType)

The problem is "_structmodulestate_global->PyStructType": 
PyState_FindModule(&_structmodule) returns NULL, _structmodulestate() calls 
PyModule_GetState(NULL) which does crash.

--

The question is why the _struct module was cleared whereas there was still a 
reference to it. Is it part of a reference cycle?

--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Executing this simple code after this commit segfaults:

from multiprocessing.pool import Pool
class A(object):
def __init__(self):
self.pool = Pool()
def __del__(self):
self.pool.close()
self.pool.join()
a = A()

[1]28019 segmentation fault  ./python.exe ../lel.py

The reason is that there is a call to PyModule_GetState with the module being 
NULL:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS 
(code=1, address=0x8)
  * frame #0: 0x0001000b9737 
python.exe`PyModule_GetState(m=0x) at moduleobject.c:565:10
frame #1: 0x0001013d5972 
_struct.cpython-39d-darwin.so`s_pack(self=0x0001032fae50, 
args=0x0001033051f8, nargs=1) at _struct.c:1858:5
frame #2: 0x0001013d4a8a 
_struct.cpython-39d-darwin.so`pack(self=0x0001013be890, 
args=0x0001033051f0, nargs=2) at _struct.c:2165:14
frame #3: 0x0001000b6e5d 
python.exe`cfunction_vectorcall_FASTCALL(func=0x0001013beb30, 
args=0x0001033051f0, nargsf=9223372036854775810, 
kwnames=0x) at methodobject.c:380:24
frame #4: 0x0001001efa89 
python.exe`_PyObject_Vectorcall(callable=0x0001013beb30, 
args=0x0001033051f0, nargsf=9223372036854775810, 
kwnames=0x) at abstract.h:106:21
frame #5: 0x0001001efbfa 
python.exe`call_function(tstate=0x000101003ec0, 
pp_stack=0x7ffeefbf6e60, oparg=2, kwnames=0x) at 
ceval.c:4984:13
frame #6: 0x0001001ea00c 
python.exe`_PyEval_EvalFrameDefault(f=0x000103305050, throwflag=0) at 
ceval.c:3465:23
frame #7: 0x0001001d8c87 
python.exe`PyEval_EvalFrameEx(f=0x000103305050, throwflag=0) at 
ceval.c:737:12
frame #8: 0x00010004374e 
python.exe`function_code_fastcall(co=0x000103165380, 
args=0x000103301400, nargs=2, globals=0x000103131d10) at call.c:293:14
frame #9: 0x00010004317a 
python.exe`_PyFunction_Vectorcall(func=0x00010324d910, 
stack=0x0001033013f0, nargsf=9223372036854775810, 
kwnames=0x) at call.c:331:20
frame #10: 0x0001001efa89 
python.exe`_PyObject_Vectorcall(callable=0x00010324d910, 
args=0x0001033013f0, nargsf=9223372036854775810, 
kwnames=0x) at abstract.h:106:21
frame #11: 0x0001001efbfa 
python.exe`call_function(tstate=0x000101003ec0, 
pp_stack=0x7ffeefbf8950, oparg=2, kwnames=0x) at 
ceval.c:4984:13
frame #12: 0x0001001ea09c 
python.exe`_PyEval_EvalFrameDefault(f=0x000103301250, throwflag=0) at 
ceval.c:3482:23
frame #13: 0x0001001d8c87 
python.exe`PyEval_EvalFrameEx(f=0x000103301250, throwflag=0) at 
ceval.c:737:12
frame #14: 0x0001001f126c 
python.exe`_PyEval_EvalCodeWithName(_co=0x000103159930, 
globals=0x000103131d10, locals=0x, args=0x0001032b2f68, 
argcount=2, kwnames=0x, kwargs=0x0001032b2f78, kwcount=0, 
kwstep=1, defs=0x000103173f18, defcount=2, kwdefs=0x, 
closure=0x, name=0x00010315c2e0, 
qualname=0x000103158430) at ceval.c:4296:14
frame #15: 0x0001000435d0 
python.exe`_PyFunction_Vectorcall(func=0x00010324d230, 
stack=0x0001032b2f68, nargsf=9223372036854775810, 
kwnames=0x) at call.c:356:12
frame #16: 0x0001001efa89 
python.exe`_PyObject_Vectorcall(callable=0x00010324d230, 
args=0x0001032b2f68, nargsf=9223372036854775810, 
kwnames=0x) at abstract.h:106:21
frame #17: 0x0001001efbfa 
python.exe`call_function(tstate=0x000101003ec0, 
pp_stack=0x7ffeefbfa610, oparg=2, kwnames=0x) at 
ceval.c:4984:13
frame #18: 0x0001001ea09c 
python.exe`_PyEval_EvalFrameDefault(f=0x0001032b2de0, throwflag=0) at 
ceval.c:3482:23
frame #19: 0x0001001d8c87 
python.exe`PyEval_EvalFrameEx(f=0x0001032b2de0, throwflag=0) at 
ceval.c:737:12
frame #20: 0x00010004374e 
python.exe`function_code_fastcall(co=0x0001031d5860, 
args=0x000103302fd8, nargs=2, globals=0x0001031d1b90) at call.c:293:14
frame #21: 0x00010004317a 
python.exe`_PyFunction_Vectorcall(func=0x000103256b90, 
stack=0x000103302fc8, nargsf=9223372036854775810, 
kwnames=0x) at call.c:331:20
frame #22: 0x0001001efa89 
python.exe`_PyObject_Vectorcall(callable=0x000103256b90, 
args=0x000103302fc8, nargsf=9223372036854775810, 
kwnames=0x) at abstract.h:106:21
frame #23: 0x0001001efbfa 
python.exe`call_function(tstate=0x000101003ec0, 
pp_stack=0x7ffeefbfc100, oparg=2, kwnames=0x) at 
ceval.c:4984:13
frame #24: 0x0001001ea09c 
python.exe`_PyEval_EvalFrameDefault(f=0x000103302e50, throwflag=0) at 
ceval.c:3482:23
frame #25: 0x0001001d8c87 
python.exe`PyEval_EvalFrameEx(f=0x000103302e50, throwflag=0) at 
ceval.c:737:12
frame #26: 

[issue38076] Make struct module PEP-384 compatible

[Christian Heimes]

Change by Christian Heimes :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
type:  -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Thomas Wouters]

Thomas Wouters  added the comment:


New changeset 4f384af067d05b16a554bfd976934fca9f87a1cf by T. Wouters (Dino 
Viehland) in branch 'master':
bpo-38076: Make struct module PEP-384 compatible (#15805)
https://github.com/python/cpython/commit/4f384af067d05b16a554bfd976934fca9f87a1cf


--
nosy: +twouters

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

Change by Dino Viehland :


--
keywords: +patch
pull_requests: +15456
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/15805

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue38076] Make struct module PEP-384 compatible

[Dino Viehland]

New submission from Dino Viehland :

Make struct module PEP-384 compatible

--
assignee: dino.viehland
components: Extension Modules
messages: 351524
nosy: dino.viehland, eric.snow
priority: normal
severity: normal
status: open
title: Make struct module PEP-384 compatible
versions: Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com