Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mysterious problems since keyring 16.0.1 (somehow caused by traceback kept in global variable) #386

Closed
Mekk opened this issue Mar 24, 2019 · 11 comments

Comments

Projects
None yet
2 participants
@Mekk
Copy link
Contributor

commented Mar 24, 2019

Long story:
https://bz.mercurial-scm.org/show_bug.cgi?id=6043
(mostly read from comment 14)

Short story: change introduced in
c778a14
caused problems when keyring is imported inside Mercurial (with it's demandimport machinery). What is more funny, we see and test those problems on Linux and Mac.

I have no idea what's exactly going on and which part of the code is really guilty, but the whole problem narrows down to traceback object being kept by keyring/backends/Windows.py in global variable (missing_deps) - most likely traceback keeps frames alive, frames keeps some variables alive etc.

Considering missing_deps is used there only as a boolean (to test whether there were any problems), please consider keeping such a flag instead. Fixes like:

with ExceptionRaisedContext() as missing_deps:
     #…

if missing_deps:
    missing_deps = True

or

with ExceptionRaisedContext() as missing_deps:
     #…

if missing_deps:
    missing_deps.traceback = None

resolve the issue.

(maybe it would be even better to reconsider keeping .traceback inside keyring.errors.ExceptionInfo, this attribute is not used for anything)

PS As I checked, this is the only place in keyring code where ExceptionInfo object is kept for a long time, other places using ExceptionRaisedContext use returned object temporarily.

@jaraco jaraco closed this in 294b9c1 Mar 24, 2019

@jaraco

This comment has been minimized.

Copy link
Owner

commented Mar 24, 2019

Can you confirm 18.0.1 fixes the issue?

@jaraco

This comment has been minimized.

Copy link
Owner

commented Mar 24, 2019

...which is still releasing at the time of this posting.

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

Unless I miss sth, you commited only change in CHANGES.rst
So it doesn't help.

@jaraco

This comment has been minimized.

Copy link
Owner

commented Mar 24, 2019

No I screwed up.

jaraco added a commit that referenced this issue Mar 24, 2019

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

9ea9bb5 helps and resolves the issue.

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

… so please fix 18.0.1 tag (if release machinery allows it), or tag 18.0.2 (if not) ;-)

@jaraco

This comment has been minimized.

Copy link
Owner

commented Mar 24, 2019

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

PyPi-installed 18.0.1 also fixes the issue.

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

What about 19.0.1 too? From my point of view this is not crucial (mercurial is not yet officially py3-migrated) but this can be faced by people devel-testing py3 version. And of course some non-hg code can also face some similar problem (which, after all, narrowed to spoiling lifecycles and maybe even creating circular refs)

@jaraco

This comment has been minimized.

Copy link
Owner

commented Mar 24, 2019

Sure. I just pushed a 19.0.1 tag.

@Mekk

This comment has been minimized.

Copy link
Contributor Author

commented Mar 24, 2019

Thank you for (very) quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.