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

Remove one UNPROTECT_PTR #3232

Closed
mattdowle opened this issue Dec 17, 2018 · 0 comments
Closed

Remove one UNPROTECT_PTR #3232

mattdowle opened this issue Dec 17, 2018 · 0 comments
Milestone

Comments

@mattdowle
Copy link
Member

@mattdowle mattdowle commented Dec 17, 2018

Tomas Kalibera contacted me :


Hi Matt,

I found data.table uses UNPROTECT_PTR. This is completely legal API, but
I discovered it is dangerously designed, I replaced it in all of base R
code and I would recommend phasing it out from all packages as well. The
problem is that one can get very nasty bugs due to interaction with
UNPROTECT. It is only really needed in generated parsers (for which I
created a new API), in all other code and also in data.table it should
be possible to replace with UNPROTECT. Could you please have a look and
update this in some next version? The problem is described in detail in
my blog post :

https://developer.r-project.org/Blog/public/2018/12/10/unprotecting-by-value/index.html

but the core problem is easy to understand: when one protects the same
pointer multiple times (which can happen..), one can accidentally
UNPROTECT_PTR the unintended one, and a following UNPROTECT may
unprotect prematurely

Thanks!
Tomas


  • We have just a single use of UNPROTECT_PTR which is good. It's in src/rbindlist.c.
@mattdowle mattdowle added this to the 1.12.0 milestone Dec 17, 2018
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.