Skip to content

Conversation

@ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented May 30, 2018

For bpo-32604 I added extra subinterpreter-related tests (see #6914), which caused a few buildbots to fail. This patch ensures that refcounts in channels are handled properly.

WIP: I'll be iterating against the failing buildbots (particularly "x86 Gentoo Refleaks 3.x") until this is resolved. The problem may have already existed before #6914 (and only surfaced with the new tests). However, I'm going to start from #6914 and then dive deeper. My first stab at the problem involves preventing a race (albeit an unlikely one) in _PyObject_GetCrossInterpreterData().

https://bugs.python.org/issue33615

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve blindly the PR just because any attempt to fix the annoying test failure is a good idea :-D Also, I trust Eric to understand his own code :-D

@vstinner
Copy link
Member

Hum, the change doesn't work yet...

test_run_string_arg_resolved (test.test__xxsubinterpreters.ChannelTests) ... Fatal Python error: Objects/dictobject.c:557 object at 0x14dbb111ec48 has negative ref count -2604246222170760230

@vstinner
Copy link
Member

Oops sorry, I forgot to mention that the crash comes from Travis CI:
https://travis-ci.org/python/cpython/jobs/385779931

@ericsnowcurrently
Copy link
Member Author

Yeah, I've been working on this whenever I can spare some time. I haven't been able to reproduce the issue locally (something racy is happening) so I was planning to rely on the buildbots to verify when I've fixed the issue. I'm actually glad things failed on CI since it reduces the latency of results (vs. buildbots). :) The current PR is the first thing I've tried to fix the issue.

Also, note that with clang the PR led to a refcount-related abort, whereas with GCC it segfaulted. I'd be much more likely to resolve this quickly if I could see a C-level backtrace on CI/buildbots (and moreso if I could repro locally), but I expect that isn't an option. :/

ericsnowcurrently added a commit that referenced this pull request May 31, 2018
… few buildbots. (gh-7288) For bpo-32604 I added some subinterpreter-related tests (see #6914) that are causing crashes on a few buildbots. I'm working on fixing the crashes (see #7251). This change temporarily disables the triggering test.
@ericsnowcurrently ericsnowcurrently force-pushed the fix-channel-test-fatal-error branch from fe153c4 to 19a530c Compare June 1, 2018 22:09
@ericsnowcurrently ericsnowcurrently force-pushed the fix-channel-test-fatal-error branch from 19a530c to ea22ea3 Compare June 1, 2018 22:52
@ericsnowcurrently
Copy link
Member Author

FYI, I'm running this against the custom buildbot builders before merging.

@ericsnowcurrently
Copy link
Member Author

The custom build looks good so I'm going to merge.

@ericsnowcurrently ericsnowcurrently changed the title [WIP] bpo-33615: avoid extra decref bpo-33615: avoid extra decref Jun 2, 2018
@ericsnowcurrently ericsnowcurrently merged commit 6379913 into python:master Jun 2, 2018
@ericsnowcurrently ericsnowcurrently deleted the fix-channel-test-fatal-error branch June 4, 2018 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants