Additional Glib::RefPtr Safety Mechanism

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Additional Glib::RefPtr Safety Mechanism

Karsten Pedersen-2
Hi all,

One area of C++ that always frustrates me is safety. Smart pointers
such as the Glib::RefPtr go a good way to avoid dangling pointers, use
after free, etc. However one area where this (and std::shared_ptr) is
lacking is that it is very easy for "this" to dangle and there really
is no protection against it. Check out a very simple example here:

https://github.com/osen/sr1/blob/master/src/tests/dangling_this.cpp

Would it interest the gtkmm project to safeguard against this? I have
prototype code (in the same repository linked above) that augments a
basic shared_ptr implementation with an "operator->" that locks the
smart pointer until it has returned.
If error such as the one above occurs it immediately aborts and
notifies the developer that things are wrong.

I have devised systems to perform the same "locking" mechanism that can
be used with *ptr, std::vector::at and iterators. Again in that same
repository you can see a number of test cases that are protected
against.

In particular I can see this being really nice to enable in "debug"
builds. Then upon a release build there is absolutely no additional
overhead (because all this extra instrumentation has been stripped
out).

What do you guys rekon? Is this idea of interest?

Best regards,

Karsten
_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Gtkmm mailing list


On Thu, 29 Aug 2019 at 21:27, Karsten Pedersen <[hidden email]> wrote:
Hi all,

One area of C++ that always frustrates me is safety. Smart pointers
such as the Glib::RefPtr go a good way to avoid dangling pointers, use
after free, etc. However one area where this (and std::shared_ptr) is
lacking is that it is very easy for "this" to dangle and there really
is no protection against it. Check out a very simple example here:

https://github.com/osen/sr1/blob/master/src/tests/dangling_this.cpp


Is this actually a common problem that needs to be solved? 

It seems like a solution in search of a problem. IMO you should just avoid using globals like this when possible, and use them carefully when necessary.

For the shared_ptr example, you can solve the problem simply by copying the global into a local variable in the one function that needs to do it. That way you don't pay the cost in every function that uses operator->

  void broken()
  {
    auto keepalive = d;
    d = std::sr1::shared_ptr<Dummy>();
    dummy = 9;
  }
 
That keeps *this alive until the end of the function.

Or slightly more efficiently:

  void broken()
  {
    auto keepalive = std::move(d);
    d = std::sr1::shared_ptr<Dummy>();
    dummy = 9;
  }




_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Gtkmm mailing list
Den 2019-08-30 kl. 11:29, skrev Jonathan Wakely via gtkmm-list:


On Thu, 29 Aug 2019 at 21:27, Karsten Pedersen <[hidden email]> wrote:
Hi all,

One area of C++ that always frustrates me is safety. Smart pointers
such as the Glib::RefPtr go a good way to avoid dangling pointers, use
after free, etc. However one area where this (and std::shared_ptr) is
lacking is that it is very easy for "this" to dangle and there really
is no protection against it. Check out a very simple example here:

https://github.com/osen/sr1/blob/master/src/tests/dangling_this.cpp


Is this actually a common problem that needs to be solved? 

It seems like a solution in search of a problem. IMO you should just avoid using globals like this when possible, and use them carefully when necessary.


I agree with Jonathan. Karsten, is this a problem you have actually seen in a program using Glib::RefPtr? Usually data of type Glib::RefPtr<SomeClass> are protected or private members of a class, or local data in a function. An almost opposite problem has been reported, though: How do you get a RefPtr to "this" (https://gitlab.gnome.org/GNOME/glibmm/issues/8).

/Kjell


_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Karsten Pedersen-2
> I agree with Jonathan. Karsten, is this a problem you have actually
> seen in a program using Glib::RefPtr? Usually data of type
> Glib::RefPtr<SomeClass> are protected or private members of a class,

I originally developed this "safety" library for my students to use as
a teaching aid (and get immediate feedback of errors if they do
something wrong rather than having to debug a corrupted heap).
So, yes, I did see this issue fairly commonly with newer or beginners
developers to C++. I understand this possibly isn't the gtkmm developer
"target market" but I could also imagine it being fairly common in
larger projects with complex hierarchies of objects (such as perhaps GUI
toolkits) and especially ones involving callbacks.

Wrapping a fiddly C library like mongoose http server
(https://github.com/cesanta/mongoose) comes to mind; the disconnect
callbacks end up triggering *after* the server object goes out of
scope, leaving potential opportunity for use after delete with "this"
if you remove the current connection from any std::vectors at that
point and it is the last reference count.

I have yet to see it in any gtkmm code (you will be pleased to hear)
but my personal opinion is that it is surely a nice idea to attempt
to make C++ 100% safe rather than having to rely purely on the skills of
a developer. This is generally the trend in Rust communities but it
seems the culture in C++ communities is that not all safety is
necessary.

> > It seems like a solution in search of a problem. IMO you should
> > just avoid using globals like this when possible, and use them
> > carefully when necessary.

Admittedly the example I crafted was part of my regression tests so was
deliberately "dumb", however even if an object isn't global, for
example it could be the last reference count contained within a
structure passed as user data into a button click callback. If that
carelessly gets blown away, "this" gets subsequently used; you are
running into undefined behaviour.

> or local data in a function. An almost opposite problem has been
> reported, though: How do you get a RefPtr to "this"
> (https://gitlab.gnome.org/GNOME/glibmm/issues/8).

I am guessing this is similar to what the
std::enable_shared_from_this<T> facility is for in std. Feels a bit
messy but that is just how C++ is I suppose ;)

No worries if you don't feel this is necessary, just thought I might
put the idea out there.

Karsten
_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Rob Pearce-2
On 31/08/2019 12:56, Karsten Pedersen wrote:
> I have yet to see it in any gtkmm code (you will be pleased to hear)
> but my personal opinion is that it is surely a nice idea to attempt
> to make C++ 100% safe rather than having to rely purely on the skills of
> a developer.
That's a nice sentiment but it's also garbage. You NEED to rely on the
skills of your developers, because they are (hopefully) intelligent
humans who know what they're trying to do. The library isn't. It can't
achieve what you want.
>   This is generally the trend in Rust communities but it
> seems the culture in C++ communities is that not all safety is
> necessary.

No, the culture in C++ communities is that safety is something you
design in, not something the universe provides for you. This is because
C++ developers live in the real world, and it's the reason code written
in Rust is mostly astonishingly bloated, inefficient, system-crippling junk.

Padded playgrounds are nice for toddlers but adults shouldn't need them.
Your safety library may be great for students (although I'm not
convinced they wouldn't learn better by having a few catastrophic
experiences) but for real-world developers it belongs in a suite of
stuff like valgrind.


_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Karsten Pedersen-2
> No, the culture in C++ communities is that safety is something you
> design in, not something the universe provides for you. This is
> because C++ developers live in the real world, and it's the reason
> code written in Rust is mostly astonishingly bloated, inefficient,
> system-crippling junk.

Indeed but as a C++ developer I am sure you know full well that mistakes
do happen. I agree with you completely about the direction of Rust but
it does provide some evidence that there are those that aren't quite
satisfied with the safety of C++ by default. The difference is that
this additional safety instrumentation needs only be present during
debug time so shouldn't even exist within the release build of some
software. Unlike Rust where it is quite wasteful.

> them. Your safety library may be great for students (although I'm not
> convinced they wouldn't learn better by having a few catastrophic
> experiences) but for real-world developers it belongs in a suite of
> stuff like valgrind.

The biggest issue is that often many of them wouldn't experience any
catastrophic problems at all. They wouldn't even know anything was
wrong with their code. It is only when I would mark it and send their
binary through Valgrind it would light up like a Christmas tree!
Undefined behaviour is frustrating like that ;)

Best regards,

Karsten

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Rob Pearce-2
On 31/08/2019 16:03, Karsten Pedersen wrote:
> The biggest issue is that often many of them wouldn't experience any
> catastrophic problems at all. They wouldn't even know anything was
> wrong with their code. It is only when I would mark it and send their
> binary through Valgrind it would light up like a Christmas tree!
> Undefined behaviour is frustrating like that;)

True, but even the best hand-holding nanny-code won't catch all the
undefined behaviour - some of it is design rather than implementation -
so it would do them good to be left floundering with a program that just
misbehaves without warning, to teach them that they will, at some point,
hit a mysterious unfathomable problem that doesn't respond to simple
static or even run-time checks. Then you can teach them about core dumps
and why they're useful ;)


_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Gtkmm mailing list
In reply to this post by Karsten Pedersen-2


On Sat, 31 Aug 2019, 11:55 Karsten Pedersen, <[hidden email]> wrote:
I have yet to see it in any gtkmm code (you will be pleased to hear)
but my personal opinion is that it is surely a nice idea to attempt
to make C++ 100% safe rather than having to rely purely on the skills of
a developer. This is generally the trend in Rust communities but it
seems the culture in C++ communities is that not all safety is
necessary.

With Rust that safety is checked and enforced by the language and the compiler. The language rules mean that most checking can have no overhead at runtime. Your solution incurs non-zero runtime cost (extra reference counting) which can't be optimised away in general.

Being 100% safe would be nice, but there's a trade off. Achieving it in C++ is either impossible or very expensive. I don't agree that "it is surely a nice idea to attempt to make C++ 100% safe". It's unrealistic to pretend otherwise.




_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list
Reply | Threaded
Open this post in threaded view
|

Re: Additional Glib::RefPtr Safety Mechanism

Damon Register-3
In reply to this post by Rob Pearce-2
On 8/31/2019 17:10, Rob Pearce wrote:
> or even run-time checks. Then you can teach them about core dumps and why they're useful ;)
I thought core memory was only in museums.?? Are there still any
computers with core memory? :-)

Damon Register

_______________________________________________
gtkmm-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtkmm-list