OK, thanks. I've forward this on to Scott Heiman who maintains the .NET version of the library. I assume it's OK to get rid of the NULL check. But I'd like to give Scott a chance to comment.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
Anonymous
-
2019-12-14
I remember reading (somewhere) that finalizers may be called more than once. Thus, I think the nullptr check is necessary.
Having said that, I have never seen a finalizer called more than once, so Markus may be correct. If someone can justify the removal of the nullptr check then I will remove them;' otherwise, I see no harm in keeping the check.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
Anonymous
-
2019-12-15
Section 15.4.5 refers to pointers to managed objects which are allocated on the CLI heap. The pointers that are tested in the finalizer functions point to unmanaged/native objects which are allocated on the native heap. I'm not convinced that 15.4.5 applies to unmanaged objects.
Which software behaviour may you expect for the deletion of objects allocated on the CLI heap according to the standard specification?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
Anonymous
-
2019-12-15
I don't know. As I stated before, the pointers that I am testing and deleting are allocated on the unmanaged heap, not the CLI heap. A better place to ask your question is on the Microsoft .NET forums.
The first link in my previous post was published by a Microsft employee who helped design .NET. He discusses the rational for implementing Finalizers and why they should be used to free unmanaged memory in managed classes. His discussion may answer your question.
I believe that I have presented enough information to justify the implementation. The only reason that I will change the implementation is
1) It is a bug that causes applications to crash or behave in an unexpected manner, or
2) It seriously degrades application performance.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The first link in my previous post was published by a Microsft employee who helped design .NET.
I find the usage of deterministic object destruction clear.
I wonder also why the author “branbray” suggested in the article “Some Notes about Mixed Types” (from 2005-07-20) to use an extra null pointer check before the C++ delete operator call in the finalizer of the class template “Embedded”.
2) It seriously degrades application performance.
How do you think about to compare run time characteristics for less questionable code?
Would you like to reuse smart pointers?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Anonymous
Anonymous
-
2019-12-15
Ok, I reread your first post and saw the link on the first line (which I did not see before). Yes, apparently the C++ standard allows you to delete null pointers. I was not aware of this.
I will remove this unneccessary check in a future release.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I've updated the dotnet branch for any changes on the .NET library. Also note that I'll require C++11 support in the next version of GeographicLib which means that VS 2013 will be the minimum supported version of Visual Studio.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
An extra null pointer check is not needed in functions like the following.
OK, thanks. I've forward this on to Scott Heiman who maintains the .NET version of the library. I assume it's OK to get rid of the NULL check. But I'd like to give Scott a chance to comment.
I remember reading (somewhere) that finalizers may be called more than once. Thus, I think the nullptr check is necessary.
Having said that, I have never seen a finalizer called more than once, so Markus may be correct. If someone can justify the removal of the nullptr check then I will remove them;' otherwise, I see no harm in keeping the check.
Section 15.4.5 refers to pointers to managed objects which are allocated on the CLI heap. The pointers that are tested in the finalizer functions point to unmanaged/native objects which are allocated on the native heap. I'm not convinced that 15.4.5 applies to unmanaged objects.
I check the unmanaged pointers because the samples that I have found on the Internet check the pointers as well.
https://blogs.msdn.microsoft.com/branbray/2005/07/20/some-notes-about-mixed-types/ search for "!Embedded".
https://communities.bentley.com/other/old_site_member_blogs/bentley_employees/b/mdanes_blog/posts/idisposable
https://www.wihlidal.com/files/getd_bonus_chp_2.pdf
Which software behaviour may you expect for the deletion of objects allocated on the CLI heap according to the standard specification?
I don't know. As I stated before, the pointers that I am testing and deleting are allocated on the unmanaged heap, not the CLI heap. A better place to ask your question is on the Microsoft .NET forums.
The first link in my previous post was published by a Microsft employee who helped design .NET. He discusses the rational for implementing Finalizers and why they should be used to free unmanaged memory in managed classes. His discussion may answer your question.
I believe that I have presented enough information to justify the implementation. The only reason that I will change the implementation is
1) It is a bug that causes applications to crash or behave in an unexpected manner, or
2) It seriously degrades application performance.
I find this information surprising.
Ok, I reread your first post and saw the link on the first line (which I did not see before). Yes, apparently the C++ standard allows you to delete null pointers. I was not aware of this.
I will remove this unneccessary check in a future release.
I've updated the dotnet branch for any changes on the .NET library. Also note that I'll require C++11 support in the next version of GeographicLib which means that VS 2013 will be the minimum supported version of Visual Studio.