[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

Change by STINNER Victor :


--
components: +Subinterpreters

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

> I close this issue with a complex history.
>
> If someone wants to continue to work on this topic, please open an issue with 
> a very clear description of what should be done and how it is supposed to be 
> used.

Yeah, there is more to do.  I'll create a new issue.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

This issue has a long history. A change has been applied and then reverted 
three times in a row. Pending calls are now per-interpreter.

The issue title is "Add a cross-interpreter-safe mechanism to indicate that an 
object may be destroyed." but I don't understand if pending calls are expected 
to be used to communicate between two interpreters. Why not using a UNIX pipe 
and exchange bytes through it? Py_AddPendingCall() is a weird concept. I would 
prefer to not abuse it.

Moreover, it's unclear if this issue attempts to *share* a same object between 
two interpreters. I would prefer to avoid that by any possible way.

I close this issue with a complex history.

If someone wants to continue to work on this topic, please open an issue with a 
very clear description of what should be done and how it is supposed to be used.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions: +Python 3.9 -Python 3.8

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> FYI, after merging that PR I realized that the COMPUTE_EVAL_BREAKER macro 
> isn't quite right.

I reworked this function in bpo-40010.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

Pavel Kostyuchenko:
> (...) The error seems to be caused not by those changes, but by lack of 
> synchronization in the multiprocessing.managers.Server.

Pavel: would you mind to open a separated issue to suggest to add 
synchronization and/or avoid daemon thread in multiprocessing?

The concurrent.futures module was recently modified to avoid daemon threads in 
bpo-39812.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

I created bpo-40231: Fix pending calls in subinterpreters.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[David Bolen]

Change by David Bolen :


--
nosy:  -db3l

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

New changeset cfc3c2f8b34d3864717ab584c5b6c260014ba55a by Victor Stinner in 
branch 'master':
bpo-37127: Remove _pending_calls.finishing (GH-19439)
https://github.com/python/cpython/commit/cfc3c2f8b34d3864717ab584c5b6c260014ba55a

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> Having interpreters interfering with each other's objects is almost certain 
> to lead to race conditions.

To be honest, I don't understand the purpose of this issue. Some messages are 
talking about pending calls, some others are talking about channels, and a few 
are talking about "shared" objects.

Can someone either clarify the exact intent of this issue, or close this one 
and create a new issue with a clear explanation of what should be done and why?

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

FYI, in bpo-39984 Victor moved pending calls to PyInterpreterState, which was 
part of my reverted change.  However, there are a few other pieces of that 
change that need to be applied before this issue is resolved.  I'm not sure 
when I'll get to it, but hopefully soon.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Mark Shannon]

Mark Shannon  added the comment:

Just to add my 2 cents.

I think this a bad idea and is likely to be unsafe.
Having interpreters interfering with each other's objects is almost certain to 
lead to race conditions.

IMO, objects should *never* be shared across interpreters (once interpreters 
are expected to be able to run in parallel).

Any channel between two interpreters should consist of two objects, one per 
interpreter. The shared memory they use to communicated can be managed by the 
runtime. Likewise for inter-interpreter locks. The lock itself should be 
managed by the runtime, with each interpreter having its own lock object with a 
handle on the shared lock.

--
nosy: +Mark.Shannon

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Paulo Henrique Silva]

Change by Paulo Henrique Silva :


--
nosy: +phsilva

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

I partially reimplemented commit ef4ac967e2f3a9a18330cc6abe14adb4bc3d0465 in 
the following issues:

* bpo-39984: Move some ceval fields from _PyRuntime.ceval to 
PyInterpreterState.ceval
* bpo-40010: Inefficient signal handling in multithreaded applications
* bpo-39877: Daemon thread is crashing in PyEval_RestoreThread() while the main 
thread is exiting the process
* bpo-37127: Handling pending calls during runtime finalization may cause 
problems.

I cannot give a list of commits, I made too many of them :-D


The most important change is:

commit 50e6e991781db761c496561a995541ca8d83ff87
Author: Victor Stinner 
Date:   Thu Mar 19 02:41:21 2020 +0100

bpo-39984: Move pending calls to PyInterpreterState (GH-19066)

If Py_AddPendingCall() is called in a subinterpreter, the function is
now scheduled to be called from the subinterpreter, rather than being
called from the main interpreter.

Each subinterpreter now has its own list of scheduled calls.

* Move pending and eval_breaker fields from _PyRuntimeState.ceval
  to PyInterpreterState.ceval.
* new_interpreter() now calls _PyEval_InitThreads() to create
  pending calls lock.
* Fix Py_AddPendingCall() for subinterpreters. It now calls
  _PyThreadState_GET() which works in a subinterpreter if the
  caller holds the GIL, and only falls back on
  PyGILState_GetThisThreadState() if _PyThreadState_GET()
  returns NULL.


My plan is now to fix pending calls in subinterpreters. Currently, they are 
only executed in the "main thread"... _PyRuntimeState.main_thread must be moved 
to PyInterpreterState, as it was done in commit 
ef4ac967e2f3a9a18330cc6abe14adb4bc3d0465. 

This issue shows that it's dangerous to change too many things at once. Python 
internals were still very fragile: bpo-39877 and and bpo-37127 are good 
examples of that. I'm trying to move *very slowly*. Move pieces one by one.

I added _Py_ThreadCanHandlePendingCalls() function in commit 
d83168854e19d0381fa57db25fca6c622917624f (bpo-40010): it should ease moving 
_PyRuntimeState.main_thread to PyInterpreterState. I also added 
_Py_ThreadCanHandleSignals() in a previous commit, so it's easier to change 
which thread is allowed or not to handle signals and pending calls.

I also plan to revisit (removal) pending_calls.finalizing added by commit 
842a2f07f2f08a935ef470bfdaeef40f87490cfc (bpo-33608). I plan to work on that in 
bpo-37127.

--
nosy: +vstinner

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Maciej Szulik]

Change by Maciej Szulik :


--
nosy: +maciej.szulik

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Thanks for the detailed analysis, Phil.  I think the results are pretty 
conclusive: daemon threads are the worst. :)  But seriously, thanks.

As you demonstrated, it isn't just Python "daemon" threads that cause the 
problem.  It is essentially any external access of the C-API once runtime 
finalization has started.  The docs [1] aren't super clear about it, but there 
are some fundamental assumptions we make about runtime finalization:

* no use of the C-API while Py_FinalizeEx() is executing (except for a few 
helpers like Py_Initialized)
* only a small portion of the C-API is available afterward (at least until 
Py_Initialize() is run)

I guess the real question is what to do about this?

Given that this is essentially a separate problem, let's move further 
discussion and effort over related to sorting out problematic threads to 
#36476, "Runtime finalization assumes all other threads have exited."  @Phil, 
would you mind attaching those same two files to that issue?


[1] https://docs.python.org/3/c-api/init.html#c.Py_FinalizeEx

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

I'm out of time and this deserves some careful discussion.  I'll get to it next 
Friday (or sooner if possible).  Sorry!

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Sorry for the delay, Phil.  I'll try to take a look in the next couple of hours.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Phil Connell]

Phil Connell  added the comment:

The attached patch (wrap_threadstate.diff) is enough to stop the crash. It's a 
slightly dirty proof-of-concept, but equally could be the basis for a solution.

The main functional issue is that there's still a race on the Py_BLOCK_THREADS 
side: it's possible that the "is threadstate still valid" check can pass, but 
the interpreter is finalised while the daemon thread is trying to reacquire the 
GIL in PyEval_RestoreThread.

(The Py_UNBLOCK_THREADS side is non-racy as the GIL is held while the ts and 
wrapper updates are done).

--
Added file: https://bugs.python.org/file48732/wrap_threadstate.diff

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Phil Connell]

Phil Connell  added the comment:

Just to summarise, I'm fairly sure this is exactly what Victor saw: a daemon 
thread attempts to reacquire the GIL via Py_END_ALLOW_THREADS after interpreter 
finalisation. Obviously the threadstate pointer held by the thread is then 
invalid...so we crash.

So I see basically two options:

1. Don't (always) free threadstate structures in Py_Finalize, and figure out a 
way to avoid leaking them (if Python is re-initialized in the same process).

2. Ban this behaviour entirely, e.g. have Py_Finalize fail if there are live 
threads with threadstate objects.

The discussion so far assumes that we should support this, i.e. #1. Any 
thoughts on that? (I'll have a think about whether this is actually doable!)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Phil Connell]

Phil Connell  added the comment:

Based on Victor's info from https://bugs.python.org/issue36114#msg337090 I 
believe the crash is essentially what's reproduced in the attached program.

>From the root of a (built) cpython clone run:

gcc -c -o fini_crash.o -IInclude -I. fini_crash.c && gcc -o fini_crash 
fini_crash.o libpython3.9.a -lcrypt -lpthread -ldl -lutil -lm && ./fini_crash

The output should be:

MAIN: allow other thread to execute 
   
OTHER: acquired GIL 
   
OTHER: released GIL 
   
MAIN: interpreter finalized
OTHER: attempt to acquire GIL...crash!
[1]266749 segmentation fault (core dumped)  ./fini_crash

And running it through valgrind:

$ valgrind --suppressions=Misc/valgrind-python.supp fini_crash  

   -- COMMAND -- 13:4[12/5973]
==266836== Memcheck, a memory error detector
==266836== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==266836== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==266836== Command: fini_crash
==266836==
MAIN: allow other thread to execute   
OTHER: acquired GIL
OTHER: released GIL 
   
MAIN: interpreter finalized
OTHER: attempt to acquire GIL...crash!  
   
==266836== Thread 2:
   
==266836== Invalid read of size 8   
   
==266836==at 0x15607D: PyEval_RestoreThread (ceval.c:389)   

   
==266836==by 0x15479F: evil_main (in /home/phconnel/dev/cpython/fini_crash)
==266836==by 0x48B94CE: start_thread (in /usr/lib/libpthread-2.30.so)
==266836==by 0x4B232D2: clone (in /usr/lib/libc-2.30.so)
==266836==  Address 0x4d17270 is 16 bytes inside a block of size 264 free'd
==266836==at 0x48399AB: free (vg_replace_malloc.c:540)
==266836==by 0x1773FF: tstate_delete_common (pystate.c:829)
==266836==by 0x1773FF: _PyThreadState_Delete (pystate.c:848)
==266836==by 0x1773FF: zapthreads (pystate.c:311)
==266836==by 0x1773FF: PyInterpreterState_Delete (pystate.c:321)
==266836==by 0x174920: finalize_interp_delete (pylifecycle.c:1242)
==266836==by 0x174920: Py_FinalizeEx.part.0 (pylifecycle.c:1400)
==266836==by 0x15487B: main (in /home/phconnel/dev/cpython/fini_crash)
==266836==  Block was alloc'd at
==266836==at 0x483877F: malloc (vg_replace_malloc.c:309)
==266836==by 0x178D7C: new_threadstate (pystate.c:557)
==266836==by 0x178D7C: PyThreadState_New (pystate.c:629)
==266836==by 0x178D7C: PyGILState_Ensure (pystate.c:1288)
==266836==by 0x154759: evil_main (in /home/phconnel/dev/cpython/fini_crash)
==266836==by 0x48B94CE: start_thread (in /usr/lib/libpthread-2.30.so)
==266836==by 0x4B232D2: clone (in /usr/lib/libc-2.30.so)
==266836== 
==266836== Invalid read of size 8
==266836==at 0x156081: PyEval_RestoreThread (ceval.c:389)
==266836==by 0x15479F: evil_main (in /home/phconnel/dev/cpython/fini_crash)
==266836==by 0x48B94CE: start_thread (in /usr/lib/libpthread-2.30.so)
==266836==by 0x4B232D2: clone (in /usr/lib/libc-2.30.so)
==266836==  Address 0x4c3a0f0 is 16 bytes inside a block of size 2,960 free'd
==266836==at 0x48399AB: free (vg_replace_malloc.c:540)
==266836==by 0x174920: finalize_interp_delete (pylifecycle.c:1242)
==266836==by 0x174920: Py_FinalizeEx.part.0 (pylifecycle.c:1400)
==266836==by 0x15487B: main (in /home/phconnel/dev/cpython/fini_crash)
==266836==  Block was alloc'd at
==266836==at 0x483877F: malloc (vg_replace_malloc.c:309)
==266836==by 0x177153: PyInterpreterState_New (pystate.c:205)
==266836==by 0x1732BF: pycore_create_interpreter (pylifecycle.c:526)
==266836==by 0x1732BF: pyinit_config.constprop.0 (pylifecycle.c:695)
==266836==by 0x1766B7: pyinit_core (pylifecycle.c:879)
==266836==by 0x1766B7: Py_InitializeFromConfig (pylifecycle.c:1055)
==266836==by 0x1766B7: Py_InitializeEx (pylifecycle.c:1093)
==266836==by 0x154801: main (in /home/phconnel/dev/cpython/fini_crash)
==266836==

--
Added file: https://bugs.python.org/file48731/fini_crash.c

___

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Phil Connell]

Change by Phil Connell :


--
nosy: +pconnell

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Also, thanks for the suggestions, Victor!

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Well, for this PR I actually disabled the execution of pending calls during 
runtime finialization.  I'd hoped it would help, but it did not. :(

Regardless, I'll need to look even more closely at what is different during 
runtime finalization with my PR.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

My bet is that multiprocessing triggers a bug in daemon threads. 
Multiprocessing is using and abusing daemon threads. For example, as I wrote, 
test_venv misuses the multiprocessing API and implicitly let Python release (or 
maybe not!) "resources" where "resources" can be processes and threads, likely 
including daemon threads.

Eric: I would love to give you access to a VM where I can reproduce the bug 
but... I failed to reproduce the bug today :-(

I would suggest to write a stress test for daemon threads:

* spawn daemon threads: 4x times threads than CPU to stress the system
* each thread would sleep a random number of milliseconds and then execute a 
few pure Python instructions
* spawns these threads, wait a random number of milliseconds, and then "exit 
Python"

The race condition is that daemon threads may or maybe run during Python 
finalization depending on the delay.

Maybe you can make the crash more likely by adding a sleep of a few *seconds* 
before the final exit. For example, at the exit of Py_RunMain().

Last time I saw the crash, it was a thread which uses a Python structure 
whereas the memory of the structure was freed (filled with a random pattern by 
debug hooks on memory allocators).

Ah, either use a debug build of Python, or use PYTHONMALLOC=debug, or -X dev 
command line option, to fill freed memory with a specific pattern, to trigger 
the crash.

... Good luck.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

What annoy me the most in this issue is that I am unable to reproduce it. Even 
directly on the FreeBSD CURRENT buildbot, I failed to reproduce rhe crash even 
when I stressed the buildbot and the test.

On the other side, when the CI runs the test suite, crashes are likely!?

These bugs are the worst.

IMHO we must make multiprocessing more deterministic. Python 3.8 is better, but 
there are still many known issues.
https://pythondev.readthedocs.io/unstable_tests.html

test_venv multiprocessing test misuses the multiprocessing issue, it emirs a 
ResourceWarning.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:


New changeset e225bebc1409bcf68db74a35ed3c31222883bf8f by Victor Stinner in 
branch 'master':
Revert "bpo-33608: Factor out a private, per-interpreter _Py_AddPendingCall(). 
(gh-13714)" (GH-13780)
https://github.com/python/cpython/commit/e225bebc1409bcf68db74a35ed3c31222883bf8f


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> FWIW, the failures before were in test_io and test_multiprocessing_spawn,
not test_asyncio.

I am not aware of a test_asyncio issue related to this issue, only crashes
in multiprocessing. I bet on a similar crash than the previous attempt,
during shutdown.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

I had really hoped to get this in for 3.8, as I may need it.  However, 
realistically I doubt it will take just a few minutes to resolve this.  With 
the beta 1 targeted for today it makes sense to revert my change. :(

(I may still try to convince our fearless RM to allow it in beta 2, but we'll 
see.)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

FWIW, the failures before were in test_io and test_multiprocessing_spawn, not 
test_asyncio.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Please wait on reverting.  I'm going to investigate.

@koobs, yeah, I'll send you a key.  Thanks! :)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> It sounds like the exactly same bug is back: same crash on same buildbots.

Since it became clear to me that the change broke multiple buildbots and nobody 
is able to investigate the issue, I wrote PR 13780 to revert the change:
https://pythondev.readthedocs.io/ci.html#revert-on-fail

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +13665
pull_request: https://github.com/python/cpython/pull/13780

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> So far so good. :)  I'll keep an eye on things and if the buildbots are still 
> happy then I'll add back _PyEval_FinishPendingCalls() in _Py_FinalizeEx() in 
> a few days.

Another crash, on Windows:
https://bugs.python.org/issue37143

It sounds like the exactly same bug is back: same crash on same buildbots.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Kubilay Kocak]

Kubilay Kocak  added the comment:

Again, happy to provide ssh access to the koobs-freebsd-current worker to help 
analysis, just PM me ssh public keys on IRC (koobs @ #python-dev freenode)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> So far so good. :)  I'll keep an eye on things and if the buildbots are still 
> happy then I'll add back _PyEval_FinishPendingCalls() in _Py_FinalizeEx() in 
> a few days.

I have no idea if it's related, but we started to see coredumps on FreeBSD:
https://bugs.python.org/issue37135

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

FYI, after merging that PR I realized that the COMPUTE_EVAL_BREAKER macro isn't 
quite right.  While the following scenario worked before, now it doesn't:

1. interpreter A: _PyEval_AddPendingCall() causes the global
   eval breaker to be set
2. interpreter B: the next pass through the eval loop uses
   COMPUTE_EVAL_BREAKER; it has no pending calls so the global
   eval breaker is unset
3. interpreter A: the next pass through the eval loop does not
   run the pending call because the eval breaker is no longer set

This really isn't a problem because the eval breaker is triggered for the GIL 
pretty frequently.  Furthermore, it won't be a problem once the GIL is 
per-interpreter (at which point we will switch to a per-interpreter eval 
breaker).

If it is important enough then I can fix it.  I even wrote up a solution. [1]  
However, I'd rather leave it alone (hence no PR).  The alternatives are more 
complicated and the situation should be relatively short-lived.

FWIW, in addition to the solution I mentioned above, I tried a few other ways:

* have a per-interpreter eval breaker in addition to the global one
* have only a per-interpreter eval breaker (the ultimate objective)
* consolidate the pending calls for every interpreter every time
  UNSIGNAL_PENDING_CALLS and UNSIGNAL_ASYNC_EXC are used

However, each has performance penalties while the branch I created does not.  I 
try to be really careful when it comes to the performance of the eval loop. :)

[1] https://github.com/ericsnowcurrently/cpython/tree/eval-breaker-shared

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

So far so good. :)  I'll keep an eye on things and if the buildbots are still 
happy then I'll add back _PyEval_FinishPendingCalls() in _Py_FinalizeEx() in a 
few days.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset 6a150bcaeb190d1731b38ab9c7a5d1a352847ddc by Eric Snow in branch 
'master':
bpo-33608: Factor out a private, per-interpreter _Py_AddPendingCall(). 
(gh-13714)
https://github.com/python/cpython/commit/6a150bcaeb190d1731b38ab9c7a5d1a352847ddc


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

I've made a few tweaks and Victor did some cleanup, so I'm going to try the PR 
again.  At first I'm also going to disable the _PyEval_FinishPendingCalls() 
call in _Py_FinalizeEx() and then enable it is a separate PR.

Also, I've opened bpo-37127 specifically to track the problem with 
_PyEval_FinishPendingCalls() during _Py_FinalizeEx(), to ensure that the 
disabled call doesn't slip through the cracks. :)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

That's really helpful, Pavel!  Thanks for investigating so thoroughly.  I'm 
going to double check all the places I've made the assumption that "tstate" 
isn't NULL and likewise for "tstate->interp".

Is there an issue open for the bug in multiprocessing?  If not, would you mind 
opening one?

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +13601
pull_request: https://github.com/python/cpython/pull/13714

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Pavel Kostyuchenko]

Pavel Kostyuchenko  added the comment:

Also it might be viable to add some assertion to verify the take_gil is not 
called with uninitialized interpreter.
I used the changes in the attachment (take_gil.assert.patch), but it produced 
errors during test_tracemalloc with f13c5c8b9401a9dc19e95d8b420ee100ac022208 . 
It happens because, during startup with invalid arguments, the interpreter is 
finalized with pymain_main->pymain_free->_PyRuntime_Finalize before the error 
is printed. However, the problem seems to be fixed for me in the last revisions 
of master branch, so I upload the diff against it.

--
Added file: https://bugs.python.org/file48334/take_gil.assert.patch

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Pavel Kostyuchenko]

Pavel Kostyuchenko  added the comment:

I was able to reproduce the error with version 
f13c5c8b9401a9dc19e95d8b420ee100ac022208 on FreeBSD 12.0 VM. The error seems to 
be caused not by those changes, but by lack of synchronization in the 
multiprocessing.managers.Server.
The failure happens when running the 
"test_shared_memory_SharedMemoryManager_basics" with high CPU load and frequent 
interrupts e.g. moving some window during test. Mostly I used the "python -m 
test --fail-env-changed test_multiprocessing_spawn -m 'WithProcessesTestS[hu]*' 
-F" command to reproduce the crash.
By analyzing core dumps I deduced that the crash happens during this call from 
the parent test process:

class BaseManager(object):
def _finalize_manager(process, address, authkey, state, _Client):
...
try:
conn = _Client(address, authkey=authkey)
try:
dispatch(conn, None, 'shutdown')
finally:
conn.close()
except Exception:
pass

Main thread in the multiprocessing child:

class Server(object):
def serve_forever(self):
...
try:
accepter = threading.Thread(target=self.accepter)
accepter.daemon = True
accepter.start()
try:
while not self.stop_event.is_set():
self.stop_event.wait(1)
except (KeyboardInterrupt, SystemExit):
pass
finally:
...
sys.exit(0)  << main thread have finished and destroyed the 
interpreter

Worker thread in the multiprocessing child.
Locals:
File "/usr/home/user/cpython/Lib/multiprocessing/managers.py", line 214, in 
handle_request
c.send(msg)
self = 
funcname = 'shutdown'
result = None
request = (None, 'shutdown', (), {})
ignore = None
args = ()
kwds = {}
msg = ('#RETURN', None)

Listing:
class Server(object):
def handle_request(self, c):
...
try:
result = func(c, *args, **kwds)  << calls Server.shutdown method
except Exception:
msg = ('#TRACEBACK', format_exc())
else:
msg = ('#RETURN', result)
try:
c.send(msg)  << crashes with SIGBUS in _send_bytes -> write -> 
take_gil -> SET_GIL_DROP_REQUEST(tstate->interp)
except Exception as e:
try:
c.send(('#TRACEBACK', format_exc()))
except Exception:
pass
...
def shutdown(self, c):
...
try:
util.debug('manager received shutdown message')
c.send(('#RETURN', None))
except:
import traceback
traceback.print_exc()
finally:
self.stop_event.set()

Worker thread is daemonic and is not terminated during the interpreter 
finalization, thus it might still be running and is terminated silently when 
the process exits. The connection (c) has different implementations on several 
platforms, so we cannot be sure whether the connection is closed during 
shutdown or not, whether the last "c.send(msg)" blocks until the end of the 
process, returns instantly, or fails inconsistently.
The error was there for a long time, but for two reasons it didn't cause much 
trouble:
- the race condition is hard to trigger;
- SET_GIL_DROP_REQUEST used to ignore the errorneous state of interpreter, but 
introduction of tstate->interp argument by Eric manifested SIGBUS on FreeBSD.

I haven't managed to find a nice clean test to reproduce the bug automatically. 
I suggest the changes for the multiprocessing/managers.py in the attachment.

--
nosy: +shprotx
Added file: https://bugs.python.org/file48333/managers.patch

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Kubilay Kocak]

Kubilay Kocak  added the comment:

@Eric Happy to give you SSH access to one of the FreeBSD buildbots that's 
experienced the crash if it helps. Just flick me a pubkey, I'm on IRC (koobs @ 
freenode / #python-dev)

--
nosy: +koobs

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Steve Dower]

Steve Dower  added the comment:


New changeset 264490797ad936868c54b3d4ceb0343e7ba4be76 by Steve Dower in branch 
'master':
bpo-33608: Normalize atomic macros so that they all expect an atomic struct 
(GH-12877)
https://github.com/python/cpython/commit/264490797ad936868c54b3d4ceb0343e7ba4be76


--
nosy: +steve.dower

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Steve Dower]

Change by Steve Dower :


--
pull_requests: +12802

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[David Bolen]

David Bolen  added the comment:

I just noticed that my last message referenced the wrong commit.  My test 
failures were against commit f13c5c8b9401a9dc19e95d8b420ee100ac022208 (the same 
as Victor).

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[David Bolen]

David Bolen  added the comment:

Eric, I'm also seeing the same Win7 and Win10 worker failures with commit 
b75b1a350 as last time (test_multiprocessing_spawn on both, and test_io on 
Win7).

For test_multiprocessing_spawn, it fails differently than Victor since no core 
file is generated, but I assume it's related in terms of child process 
termination.  See for example 
https://buildbot.python.org/all/#/builders/3/builds/2390 for Win10, where 
test_mymanager_context fails with:

==
FAIL: test_mymanager_context 
(test.test_multiprocessing_spawn.WithManagerTestMyManager)
--
Traceback (most recent call last):
  File 
"D:\buildarea\3.x.bolen-windows10\build\lib\test\_test_multiprocessing.py", 
line 2747, in test_mymanager_context
self.assertIn(manager._process.exitcode, (0, -signal.SIGTERM))
AssertionError: 3221225477 not found in (0, -15)
--

(3221225477 is C005 which I believe is an access violation)

For some reason, the Windows 7 worker didn't get a test run while your commit 
was live, but I can reproduce the same error manually.

For test_io, as before, its a shutdown lock failure:

==
FAIL: test_daemon_threads_shutdown_stdout_deadlock (test.test_io.CMiscIOTest)
--
Traceback (most recent call last):
  File "D:\cygwin\home\db3l\python.test\lib\test\test_io.py", line 4157, in 
test_daemon_threads_shutdown_stdout_deadlock
self.check_daemon_threads_shutdown_deadlock('stdout')
  File "D:\cygwin\home\db3l\python.test\lib\test\test_io.py", line 4148, in 
check_daemon_threads_shutdown_deadlock
self.assertIn("Fatal Python error: could not acquire lock "
AssertionError: "Fatal Python error: could not acquire lock for 
<_io.BufferedWriter name=''> at interpreter shutdown, possibly due to 
daemon threads" not found in ''

--

In manual attempts I have yet to be able to recreate the 
test_multiprocessing_spawn failure under Win10 but can usually manage a 25-50% 
failure rate under Win7 (which is much slower).  The test_io failure on Win7 
however, appears to be more easily reproducible.

It's possible I/O is more critical than CPU, or perhaps its impact on latency; 
I seem to more easily exacerbate the test_multiprocessing_spawn failure rate by 
loading down the host disk than its CPU.  I also noticed that the Win10 failure 
was when test_io and test_multiprocessing_spawn overlapped.

While I'm guessing this should happen on any low powered Windows VM, if it 
would help, I could arrange remote access to the Win7 worker for you.  Or just 
test a change on your behalf.  In fairness, it's unlikely to be useful for any 
significant remote debugging but perhaps at least having somewhere you could 
test a change, even if just with print-based debugging, might help.  And while 
it might be an independent issue, the test_io failure rate appears to occur 
more reliably than test_multiprocessing_spawn.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

> Any ideas of how far I need to throttle the VM?  Is there more than just CPU 
> that I need to limit?

I don't know how to make the race condition more likely. I'm not sure that 
starving the CPU helps. Maybe try the opposite: add more CPUs and reduce the 
number of tests run in parallel.

Did you test commit f13c5c8b9401a9dc19e95d8b420ee100ac022208?

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

@Victor, I set up a FreeBSD 12.0 VM (in Hyper-v) and made sure core files were 
getting generated for segfaults.  Then I cloned the cpython repo, built it 
(using GCC), and ran regrtest as you recommended.  It generated no core files 
after half an hour.  I adjusted the VM down to 1 CPU from 4 and there were no 
segfaults over an hour and a half of running those 4 test loops.  So I've set 
the VM to 10% of a CPU and still have gotten no core files after over half an 
hour.  The load average has been hovering between 5 and 6.  I guess I'm not 
starving the VM enough. :)

Any ideas of how far I need to throttle the VM?  Is there more than just CPU 
that I need to limit?

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

FYI AMD64 FreeBSD CURRENT Shared 3.x failed at commit 
f13c5c8b9401a9dc19e95d8b420ee100ac022208:
https://buildbot.python.org/all/#/builders/168/builds/913

But this issue has already been fixed: Eric reverted his change.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

I tried but failed to reproduce the crash on Linux!?

$ sudo bash -c 'echo "%e.%p" > /proc/sys/kernel/core_pattern'
$ ./python -m test --matchfile=bisect5 test_multiprocessing_spawn 
--fail-env-changed -F
# wait 5 min
^C
$ ./python -m test --matchfile=bisect5 -j0 test_multiprocessing_spawn 
--fail-env-changed -F  # I added -j0
# wait 5 min
^C
$ ./python -m test --matchfile=bisect5 -j0 test_multiprocessing_spawn 
--fail-env-changed -F  # I added -j0
# wait 5 min
^C

No coredump seen...

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset b75b1a3504a0cea6fac6ecba44c10b2629577025 by Eric Snow in branch 
'master':
bpo-33608: Revert "Factor out a private, per-interpreter _Py_AddPendingCall()." 
(gh-12806)
https://github.com/python/cpython/commit/b75b1a3504a0cea6fac6ecba44c10b2629577025


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

Eric Snow:
> For now I'll revert.  This is not code that changes very often, so there 
> isn't much benefit to keeping it merged.  Testing against a separate branch 
> is just as easy.

Again, Python shutdown is *really* fragile. Last time I tried to "enhance" it, 
I also introduced random regressions and so I had to revert my changes.

Old info about the crash, should still be relevant:
https://bugs.python.org/issue36114#msg337090

> Could you point me at an immage for that VM or instructions on how to 
> reproduce it?  I hate having to bother you to test my changes! :)

*In theory*, you should be able to reproduce the crash on any platform. But in 
my experience, bugs which involve multiple threads are simply "more likely" on 
FreeBSD because FreeBSD manages threads very differently than Linux. Sometimes, 
a bug can only be reproduce on one specific FreeBSD computer, but once the root 
issue has been identified, we start to be able to trigger the crash reliably on 
other platforms (like Linux).

My procedure to reproduce the crash on FreeBSD:
https://bugs.python.org/issue36114#msg337092

I'm using FreeBSD 12.0 RELEASE VM hosted on Linux. My FreeBSD is not customized 
in any way.

On modern Linux distributions, coredumps are no longer written in the current 
directory but handled by a system service like ABRT on Fedora. For this reason, 
Python test runner can "miss" crashes, especially in child processes run by 
tests (not directly in the process used to run the test).

To get a coredump in the current directory on Linux, you can use:

sudo bash -c 'echo "%e.%p" > /proc/sys/kernel/core_pattern'

Manual test:

$ ./python -c 'import ctypes; ctypes.string_at(0)'
Segmentation fault (core dumped)

vstinner@apu$ git status
...
Untracked files:
python.18343
...

Say hello to python.18343 coredump!


Usually, running the command which trigger the crash multiple times in parallel 
(in different terminals, using screen and multiple terminals, etc.) makes the 
crash more likely since it does stress the system.

Sometimes, I run the Python test suite in parallel to stress the system even 
more.

The goal of the game is to trigger a race condition which depends on time. 
Stressing the system helps to "randomize" timings.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12733

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

Thanks for checking, Victor.  Don't feel bad about your results, nor about not 
checking sooner. :)  We'll get this sorted out.

For now I'll revert.  This is not code that changes very often, so there isn't 
much benefit to keeping it merged.  Testing against a separate branch is just 
as easy.

Could you point me at an immage for that VM or instructions on how to reproduce 
it?  I hate having to bother you to test my changes! :)

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

I ran "./python -m test --matchfile=bisect5 test_multiprocessing_spawn 
--fail-env-changed -F" 4 times in parallel: in less than 5 minutes (in fact, I 
didn't look carefully at the terminal, maybe it was faster), I got 3 core dumps 
:-(

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

I have a bad news for you Eric: I'm able again to reproduce the crash at commit 
f13c5c8b9401a9dc19e95d8b420ee100ac022208.


vstinner@freebsd$ ./python -m test --matchfile=bisect5 
test_multiprocessing_spawn --fail-env-changed -F
Run tests sequentially
0:00:00 load avg: 0.69 [  1] test_multiprocessing_spawn
0:00:06 load avg: 0.80 [  2] test_multiprocessing_spawn
0:00:12 load avg: 1.19 [  3] test_multiprocessing_spawn
...
0:01:55 load avg: 1.48 [ 21] test_multiprocessing_spawn
0:02:01 load avg: 1.53 [ 22] test_multiprocessing_spawn
0:02:08 load avg: 1.29 [ 23] test_multiprocessing_spawn
0:02:17 load avg: 1.51 [ 24] test_multiprocessing_spawn
0:02:27 load avg: 2.27 [ 25] test_multiprocessing_spawn
0:02:38 load avg: 3.14 [ 26] test_multiprocessing_spawn
0:02:48 load avg: 3.51 [ 27] test_multiprocessing_spawn
Warning -- files was modified by test_multiprocessing_spawn
  Before: []
  After:  ['python.core'] 
test_multiprocessing_spawn failed (env changed)

== Tests result: ENV CHANGED ==

All 26 tests OK.

1 test altered the execution environment:
test_multiprocessing_spawn

Total duration: 2 min 59 sec
Tests result: ENV CHANGED


Note: Sorry for not testing before, after one long week, I didn't succeed to 
catch up with my very long list of unread emails.

I don't know what should be done. Revert? I don't have the bandwidth to 
investigate this crash.

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset f13c5c8b9401a9dc19e95d8b420ee100ac022208 by Eric Snow in branch 
'master':
bpo-33608: Factor out a private, per-interpreter _Py_AddPendingCall(). 
(gh-12360)
https://github.com/python/cpython/commit/f13c5c8b9401a9dc19e95d8b420ee100ac022208


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Paul Monson]

Change by Paul Monson :


--
pull_requests: +12593

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12327

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset 842a2f07f2f08a935ef470bfdaeef40f87490cfc by Eric Snow in branch 
'master':
bpo-33608: Deal with pending calls relative to runtime shutdown. (gh-12246)
https://github.com/python/cpython/commit/842a2f07f2f08a935ef470bfdaeef40f87490cfc


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:


New changeset e3f4070aee6f2d489416fdcafd51d6b04d661919 by Victor Stinner in 
branch 'master':
bpo-33608: Fix PyEval_InitThreads() warning (GH-12346)
https://github.com/python/cpython/commit/e3f4070aee6f2d489416fdcafd51d6b04d661919


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +12312

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset 8479a3426eb7d1840473f7788e639954363ed37e by Eric Snow in branch 
'master':
bpo-33608: Make sure locks in the runtime are properly re-created.  (gh-12245)
https://github.com/python/cpython/commit/8479a3426eb7d1840473f7788e639954363ed37e


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:


New changeset 5be45a6105d656c551adeee7770afdc3b806fbb5 by Eric Snow in branch 
'master':
bpo-33608: Minor cleanup related to pending calls. (gh-12247)
https://github.com/python/cpython/commit/5be45a6105d656c551adeee7770afdc3b806fbb5


--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12236

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12235

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12234

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12231

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Change by Eric Snow :


--
pull_requests: +12228
stage:  -> patch review

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

The bug is hard to reproduce even manually. I can test a PR for you once
it's ready.

--
title: [subinterpreters] Add a cross-interpreter-safe mechanism to indicate 
that an object may be destroyed. -> Add a cross-interpreter-safe mechanism to 
indicate that an object may be destroyed.

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[pmpp]

Change by pmpp :


--
nosy: +pmpp

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[STINNER Victor]

STINNER Victor  added the comment:

"That is relatively straight-forward except in one case: indicating that the 
other interpreter doesn't need the object to exist any more (similar to 
PyBuffer_Release() but more general)"

Why an interpreter would access an object from a different interpreter? Each 
interpreter should have its own object space, no?

--

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

Eric Snow  added the comment:

As a lesser (IMHO) alternative, we could also modify Py_DECREF
to respect a new "shared" marker of some sort (perhaps relative
to #33607), but that would probably still require one of the
refcount-based solutions (and add a branch to Py_DECREF).

--
versions: +Python 3.8 -Python 3.4

___
Python tracker 

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

[issue33608] Add a cross-interpreter-safe mechanism to indicate that an object may be destroyed.

[Eric Snow]

New submission from Eric Snow :

In order to keep subinterpreters properly isolated, objects
from one interpreter should not be used in C-API calls in
another interpreter.  That is relatively straight-forward
except in one case: indicating that the other interpreter
doesn't need the object to exist any more (similar to
PyBuffer_Release() but more general).  I consider the
following solutions to be the most viable.  Both make use
of recounts to protect cross-interpreter usage (with incref
before sharing).

1. manually switch interpreters (new private function)
  a. acquire the GIL
  b. if refcount > 1 then decref and release the GIL
  c. switch
  d. new thread (or re-use dedicated one)
  e. decref
  f. kill thread
  g. switch back
  h. release the GIL
2. change pending call mechanism (see Py_AddPendingCall) to
   per-interpreter instead of global (add "interp" arg to
   signature of new private C-API function)
  a. queue a function that decrefs the object
3. new cross-interpreter-specific private C-API function
  a. queue the object for decref (a la Py_AddPendingCall)
 in owning interpreter

I favor #2, since it is more generally applicable.  #3 would
probably be built on #2 anyway.  #1 is relatively inefficient.
With #2, Py_AddPendingCall() would become a simple wrapper
around the new private function.

--
messages: 317333
nosy: eric.snow, ncoghlan, serhiy.storchaka, vstinner, yselivanov
priority: normal
severity: normal
stage: needs patch
status: open
title: Add a cross-interpreter-safe mechanism to indicate that an object may be 
destroyed.
versions: Python 3.4

___
Python tracker 

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