Opened 8 years ago

Closed 8 years ago

#4260 defect closed duplicate (duplicate)

change twisted.python.failure.Failure stack-saving semantics

Reported by: amckinley22 Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: ivank, ghazel Branch:
Author:

Description

Failure() currently serializes the scoped variables available in a traceback by invoking twisted.reflect.safe_repr(). This protects Twisted from circular references in the traceback, but has the unfortunate side effect of serializing a potentially large amount of stuff (basically, whatever was in scope when the exception was raised). I discovered this behavior after noticing that whenever my twisted process threw an exception, the entire reactor would hang for several seconds. After lots of debugging, I figured out that a very large cache object was being serialized on every exception, just because it happened to be in scope when the exception was raised.

I've attached a patch that modifies Failure.cleanFailure() to completely strip the locals/globals from Failure.stack and Failure.frame, instead of rendering them to strings. I've also added a deprecation warning when invoking Failure.printDetailedTraceback(), because that appears to be the only code that uses that info.

Attachments (2)

failure_tback_change.diff (3.5 KB) - added by amckinley22 8 years ago.
failure_tback_change-clean.diff (3.5 KB) - added by ivank 8 years ago.
same as failure_tback_change.diff but whitespace changes removed

Download all attachments as: .zip

Change History (9)

Changed 8 years ago by amckinley22

Attachment: failure_tback_change.diff added

comment:1 Changed 8 years ago by ivank

Cc: ivank added

comment:2 Changed 8 years ago by ghazel

Cc: ghazel added

Changed 8 years ago by ivank

same as failure_tback_change.diff but whitespace changes removed

comment:3 Changed 8 years ago by ivank

There was some earlier discussion of this problem in #2466

comment:4 Changed 8 years ago by ivank

Keywords: review added

comment:5 Changed 8 years ago by therve

Keywords: review removed

comment:6 Changed 8 years ago by therve

Resolution: duplicate
Status: newclosed

So I'll close this bug as duplicate of #2466. This approach is probably more reasonable than switching from safe_repr to safe_str, but we should take into account backward compatibility in some way. The warning is a first nice step, but it should also preserve the behavior. After that, we probably need a global flag to activate the new behavior.

comment:7 Changed 7 years ago by <automation>

Owner: Glyph deleted
Note: See TracTickets for help on using tickets.