Skip to content

Fix recursion error in repr method of report thread#4176

Merged
lukasmasuch merged 2 commits intostreamlit:developfrom
lukasmasuch:fix/4172
Dec 15, 2021
Merged

Fix recursion error in repr method of report thread#4176
lukasmasuch merged 2 commits intostreamlit:developfrom
lukasmasuch:fix/4172

Conversation

@lukasmasuch
Copy link
Copy Markdown
Collaborator

@lukasmasuch lukasmasuch commented Dec 14, 2021

📚 Context

If str or repr is called on a ReportThread instance, a RecursionError will be thrown. The reason is that ReportThread contains reference to ScriptRunner (in _target) and ScriptRunner contains a reference to the ReportThread in _script_thread. This is causing a recursion.

  • What kind of change does this PR introduce?

    • Bugfix
    • Feature
    • Refactoring
    • Other, please describe:

🧠 Description of Changes

  • Removed the __repr__ implementation from ReportThread since it is not usable because of the self-reference causing a RecursionError.

    • This is a breaking API change
    • This is a visible (user-facing) change

Code Snippet:

import streamlit as st
import threading

thread = threading.current_thread()
st.info('Weird case %s' % thread)

🧪 Testing Done

  • Screenshots included
  • Added/Updated unit tests
  • Added/Updated e2e tests

🌐 References

Does this depend on other work, documents, or tickets?

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@lukasmasuch lukasmasuch changed the title Fix recursion error repr method of report thread Fix recursion error in repr method of report thread Dec 14, 2021
vdonato
vdonato approved these changes Dec 15, 2021
View reviewed changes
Copy link
Copy Markdown
Collaborator

@vdonato vdonato left a comment

Choose a reason for hiding this comment

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

🎉

@lukasmasuch lukasmasuch merged commit 4715085 into streamlit:develop Dec 15, 2021
tconkling added a commit to tconkling/streamlit that referenced this pull request Dec 20, 2021
@tconkling
Merge branch 'develop' into tim/CacheClearing
7c835d2 
* develop:
  Update error message for redacted exception (streamlit#4200)
  Release 1.3.0 (streamlit#4192)
  Wait for enough elements to load in flaky e2e tests (streamlit#4193)
  Ignore type errors introduced by mypy 0.920 (streamlit#4191)
  Change st.plotly docstring to fix docs rendering (streamlit#4182)
  Fix recursion error in repr method of report thread (streamlit#4176)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdonato vdonato vdonato approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Calling repr on current thread leads to RecursionError

2 participants

@lukasmasuch @vdonato