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

Bleadperl v5.21.5-396-ga635561 breaks Test::LeakTrace #14250

Closed
p5pRT opened this issue Nov 17, 2014 · 5 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 17, 2014

Migrated from rt.perl.org#123223 (status was 'resolved')

Searchable as RT123223$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 17, 2014

From @cpansprout

SVs_OBJECT now has a meaning on pad names, other than its meaning elsewhere.

Test​::LeakTrace allows a leaked scalar (even a pad name) to be passed to user code. And that allows pad names to reach all sorts of code paths that would not reach under usual circumstances.

I don’t want to break Test​::LeakTrace, since it is an extremely useful module that I have even used to track down core bugs.

I could change the definition of SvOBJECT, and have it check the pad name flags, too. But that would make all uses of SvOBJECT slightly slower. I wouldn’t mind doing that, though, if there were no other obvious solution.

For me, the obvious fix is to separate pad names from SVs. I have talked about this several times in the past, but never actually gotten to it. I think it needs to be done anyway, because the conflation makes both pad names and SVs more complex than they need to be.

I have outlined how the pad names could be done in <http​://www.nntp.perl.org/group/perl.perl5.porters/;msgid=20141030152505.13408.qmail@​lists-nntp.develooper.com>.
--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 17, 2014

From @iabyn

On Sun, Nov 16, 2014 at 04​:49​:36PM -0800, Father Chrysostomos wrote​:

For me, the obvious fix is to separate pad names from SVs.

+1

--
This email is confidential, and now that you have read it you are legally
obliged to shoot yourself. Or shoot a lawyer, if you prefer. If you have
received this email in error, place it in its original wrapping and return
for a full refund. By opening this email, you accept that Elvis lives.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 17, 2014

The RT System itself - Status changed from 'new' to 'open'

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 1, 2014

From @cpansprout

As of d486036, Test​::LeakTrace no longer fails its tests.

--

Father Chrysostomos

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 1, 2014

@cpansprout - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Dec 1, 2014
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.