bpo-26806: add 30 to the recursion limit in IDLE's shell #13944

taleinat · 2019-06-10T19:50:58Z

This is done to compensate for the extra stack frames added by IDLE's shell, which can cause problems when setting the recursion limit to low values.

This wraps sys.setrecursionlimit() and sys.getrecursionlimit(), adding a note about this to their doc-strings.

https://bugs.python.org/issue26806

This is done to compensate for the extra stack frames added by IDLE itself, which cause problems when setting the recursion limit to low values. This wraps sys.setrecursionlimit() and sys.getrecursionlimit() as invisibly as possible.

taleinat · 2019-06-10T20:29:44Z

@terryjreedy, I couldn't figure out where you'd like this mentioned in which docs. I'm happy to add appropriate wording if you point me in the right direction.

terryjreedy

Also see new comment on the issue, which includes some manual tests.

Lib/idlelib/run.py

bedevere-bot · 2019-06-10T20:38:06Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

terryjreedy · 2019-06-10T20:42:30Z

Our messages crossed. I worry about the docstring and doc and blurb when we agree on everything else. I considered the PRs you and Cheryl worked on last week a higher priority.

1. define wrapping code at the module level 2. increase recursion limit delta from 25 to 30 3. add a note to the wrapped functions' doc-strings 4. extract the recursion limit delta to a variable 5. rework test and add another one

taleinat · 2019-06-11T07:24:17Z

@terryjreedy, I have made the requested changes; please review again.

bedevere-bot · 2019-06-11T07:24:20Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

taleinat · 2019-07-03T07:13:00Z

Ping, @terryjreedy. I'd like to get this into 3.8.0.

BTW, should we backport this to 3.7.4?

taleinat · 2019-07-05T22:11:56Z

Doc/library/idle.rst

@@ -713,6 +713,10 @@ or ``print`` or ``write`` to sys.stdout or sys.stderr,
 IDLE should be started in a command line window.  The secondary subprocess
 will then be attached to that window for input and output.

+The IDLE code running in the execution process adds frames to the call stack
+that would not be there otherwise.  IDLE wraps ``sys.getrecursionlimit`` and
+``sys.setrecursionlimit`` to reduce their visibility.


"their" is ambiguous here; I suggest "to reduce the visibility of the additional stack frames."

Also, perhaps we should mention that this is not just about visibility? E.g. "to reduce the effect of the additional stack frames".

taleinat · 2019-07-05T22:14:36Z

@terryjreedy, see my two comments on your recent changes.

terryjreedy · 2019-07-06T07:31:10Z

I have made changes (improvements) in response to both comments. I'd like to merge this and move on to other issues.

taleinat · 2019-07-06T07:35:25Z

Looks good to me, just one minor typo. I'll fix it and merge later today even I'm at a computer.

…

On Sat, Jul 6, 2019, 10:31 Terry Jan Reedy ***@***.***> wrote: I have made changes (improvements) in response to both comments. I'd like to merge this and move on to other issues. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#13944?email_source=notifications&email_token=AAEB6OO5UUE4J3JX2UPNVOLP6BC65A5CNFSM4HWYA6G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZKUI7A#issuecomment-508904572>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEB6OLBEVI7FF47OEYJGTLP6BC65ANCNFSM4HWYA6GQ> .

miss-islington · 2019-07-06T12:35:27Z

Thanks @taleinat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

bedevere-bot · 2019-07-06T12:35:39Z

GH-14621 is a backport of this pull request to the 3.8 branch.

) This is done to compensate for the extra stack frames added by IDLE itself, which cause problems when setting the recursion limit to low values. This wraps sys.setrecursionlimit() and sys.getrecursionlimit() as invisibly as possible. (cherry picked from commit fcf1d00) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

bedevere-bot · 2019-07-06T12:35:46Z

GH-14622 is a backport of this pull request to the 3.7 branch.

) This is done to compensate for the extra stack frames added by IDLE itself, which cause problems when setting the recursion limit to low values. This wraps sys.setrecursionlimit() and sys.getrecursionlimit() as invisibly as possible. (cherry picked from commit fcf1d00) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

This is done to compensate for the extra stack frames added by IDLE itself, which cause problems when setting the recursion limit to low values. This wraps sys.setrecursionlimit() and sys.getrecursionlimit() as invisibly as possible. (cherry picked from commit fcf1d00) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

terryjreedy · 2019-07-06T16:34:18Z

2.7.4?
This will be in 2.7.5. I don't think a cherry-pick request would be worth the cost.

taleinat · 2019-07-06T18:46:32Z

2.7.4?

No, I did really mean to ask about 3.7.4, but we've already backported to 3.7, so I'll take that as a yes :)

This will be in 2.7.5. I don't think a cherry-pick request would be worth the cost.

I agree.

ZackerySpytz · 2019-07-09T05:10:00Z

@taleinat @terryjreedy It seems that IDLE fails to start after fcf1d00 when CPython is configured without docstrings (./configure --without-doc-strings). When I run ./python -m idlelib.idle, I see the following:


----------------------------------------
Unhandled server exception!
Thread: SockThread
Client Address:  ('127.0.0.1', 40895)
Request:  <socket.socket fd=3, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('127.0.0.1', 40832), raddr=('127.0.0.1', 40895)>
Traceback (most recent call last):
  File "/home/lubuntu2/cpython/Lib/socketserver.py", line 316, in _handle_request_noblock
    self.process_request(request, client_address)
  File "/home/lubuntu2/cpython/Lib/socketserver.py", line 347, in process_request
    self.finish_request(request, client_address)
  File "/home/lubuntu2/cpython/Lib/socketserver.py", line 360, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 514, in __init__
    socketserver.BaseRequestHandler.__init__(self, sock, addr, svr)
  File "/home/lubuntu2/cpython/Lib/socketserver.py", line 720, in __init__
    self.handle()
  File "/home/lubuntu2/cpython/Lib/idlelib/run.py", line 511, in handle
    install_recursionlimit_wrappers()
  File "/home/lubuntu2/cpython/Lib/idlelib/run.py", line 332, in install_recursionlimit_wrappers
    setrecursionlimit.__doc__ += "\n\n" + textwrap.fill(textwrap.dedent(f"""\
TypeError: unsupported operand type(s) for +=: 'NoneType' and 'str'

*** Unrecoverable, server exiting!
----------------------------------------
Traceback (most recent call last):
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 432, in pollresponse
    message = self.pollmessage(wait)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 384, in pollmessage
    packet = self.pollpacket(wait)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 363, in pollpacket
    raise EOFError
EOFError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/lubuntu2/cpython/Lib/runpy.py", line 192, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/lubuntu2/cpython/Lib/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/lubuntu2/cpython/Lib/idlelib/idle.py", line 14, in <module>
    main()
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 1521, in main
    shell = flist.open_shell()
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 330, in open_shell
    if not self.pyshell.begin():
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 1050, in begin
    client = self.interp.start_subprocess()
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 463, in start_subprocess
    self.transfer_path(with_cwd=True)
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 553, in transfer_path
    self.runcommand("""if 1:
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 756, in runcommand
    self.rpcclt.remotequeue("exec", "runcode", (code,), {})
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 224, in remotequeue
    return self.asyncreturn(seq)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 248, in asyncreturn
    response = self.getresponse(seq, wait=0.05)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 291, in getresponse
    response = self._getresponse(myseq, wait)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 311, in _getresponse
    response = self.pollresponse(myseq, wait)
  File "/home/lubuntu2/cpython/Lib/idlelib/rpc.py", line 436, in pollresponse
    self.handle_EOF()
  File "/home/lubuntu2/cpython/Lib/idlelib/pyshell.py", line 388, in handle_EOF
    raise EOFError
EOFError

terryjreedy · 2019-07-09T05:29:57Z

Having read the IDLE part of #14592 a couple of days before merging, it should have occurred to me that we were adding a bug. The possibility of such an oversight is why I don't try to squeeze in substantial code patched after, or even immediately before a .rc. Fortunately, beginner users of IDLE are unlikely to recompile this way. I just tested the effect of -OO and it also sets doc to None, although only for functions in the code being run, not builtins. I am working on a patch now: #14657.

) This is done to compensate for the extra stack frames added by IDLE itself, which cause problems when setting the recursion limit to low values. This wraps sys.setrecursionlimit() and sys.getrecursionlimit() as invisibly as possible.

taleinat added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels Jun 10, 2019

taleinat requested a review from terryjreedy June 10, 2019 19:50

the-knights-who-say-ni added the CLA signed label Jun 10, 2019

bedevere-bot added the awaiting core review label Jun 10, 2019

add a test + bugfix

7d7b622

taleinat force-pushed the bpo-26806/IDLE-recursion-limit branch from c7e1a04 to 7d7b622 Compare June 10, 2019 20:24

add docstring + comment referencing the bpo issue

f3b93f5

terryjreedy requested changes Jun 10, 2019

View reviewed changes

Lib/idlelib/run.py Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting core review labels Jun 10, 2019

address review comments

af3ee2b

1. define wrapping code at the module level 2. increase recursion limit delta from 25 to 30 3. add a note to the wrapped functions' doc-strings 4. extract the recursion limit delta to a variable 5. rework test and add another one

taleinat changed the title ~~bpo-26806: transparently add 25 to the recursion limit in IDLE's shell~~ bpo-26806: add 25 to the recursion limit in IDLE's shell Jun 11, 2019

taleinat changed the title ~~bpo-26806: add 25 to the recursion limit in IDLE's shell~~ bpo-26806: add 30 to the recursion limit in IDLE's shell Jun 11, 2019

bedevere-bot added awaiting change review and removed awaiting changes labels Jun 11, 2019

taleinat added 2 commits June 11, 2019 10:33

minor fix to uninstall_recursionlimit_wrappers()

9ddd699

update NEWS

38bab8d

taleinat force-pushed the bpo-26806/IDLE-recursion-limit branch from 6554218 to 38bab8d Compare June 11, 2019 07:36

terryjreedy added 3 commits July 4, 2019 21:08

Merge remote-tracking branch 'origin/master' into pr_13944

57c8b7b

Use f-string for error message and test bad calls.

6f6ca69

Add test note.

ca15c75

bedevere-bot added the awaiting merge label Jul 5, 2019

taleinat commented Jul 5, 2019

View reviewed changes

terryjreedy added 2 commits July 6, 2019 03:27

Update idle.rst

6c20f8c

Update run.py

e7427ee

taleinat added 3 commits July 6, 2019 11:11

fix typo

fd05a6a

fix whitespace

2c1d8d5

another whitespace fix

7d6c545

taleinat merged commit fcf1d00 into python:master Jul 6, 2019

bedevere-bot removed awaiting merge needs backport to 3.8 labels Jul 6, 2019

bedevere-bot removed the needs backport to 3.7 label Jul 6, 2019

taleinat deleted the bpo-26806/IDLE-recursion-limit branch July 6, 2019 12:55

terryjreedy mentioned this pull request Jul 9, 2019

bpo-26806: IDLE should run without docstrings #14657

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-26806: add 30 to the recursion limit in IDLE's shell #13944

bpo-26806: add 30 to the recursion limit in IDLE's shell #13944

taleinat commented Jun 10, 2019 •

edited

Loading

taleinat commented Jun 10, 2019

terryjreedy left a comment

bedevere-bot commented Jun 10, 2019

terryjreedy commented Jun 10, 2019

taleinat commented Jun 11, 2019

bedevere-bot commented Jun 11, 2019

taleinat commented Jul 3, 2019

taleinat Jul 5, 2019

taleinat commented Jul 5, 2019

terryjreedy commented Jul 6, 2019

taleinat commented Jul 6, 2019 via email

miss-islington commented Jul 6, 2019

bedevere-bot commented Jul 6, 2019

bedevere-bot commented Jul 6, 2019

terryjreedy commented Jul 6, 2019

taleinat commented Jul 6, 2019

ZackerySpytz commented Jul 9, 2019

terryjreedy commented Jul 9, 2019 •

edited

Loading

bpo-26806: add 30 to the recursion limit in IDLE's shell #13944

bpo-26806: add 30 to the recursion limit in IDLE's shell #13944

Conversation

taleinat commented Jun 10, 2019 • edited Loading

taleinat commented Jun 10, 2019

terryjreedy left a comment

Choose a reason for hiding this comment

bedevere-bot commented Jun 10, 2019

terryjreedy commented Jun 10, 2019

taleinat commented Jun 11, 2019

bedevere-bot commented Jun 11, 2019

taleinat commented Jul 3, 2019

taleinat Jul 5, 2019

Choose a reason for hiding this comment

taleinat commented Jul 5, 2019

terryjreedy commented Jul 6, 2019

taleinat commented Jul 6, 2019 via email

miss-islington commented Jul 6, 2019

bedevere-bot commented Jul 6, 2019

bedevere-bot commented Jul 6, 2019

terryjreedy commented Jul 6, 2019

taleinat commented Jul 6, 2019

ZackerySpytz commented Jul 9, 2019

terryjreedy commented Jul 9, 2019 • edited Loading

taleinat commented Jun 10, 2019 •

edited

Loading

terryjreedy commented Jul 9, 2019 •

edited

Loading