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

Add explicit finalizer signature to XPtr #1108

Closed
eddelbuettel opened this issue Aug 15, 2020 · 11 comments · Fixed by #1180
Closed

Add explicit finalizer signature to XPtr #1108

eddelbuettel opened this issue Aug 15, 2020 · 11 comments · Fixed by #1180

Comments

@eddelbuettel
Copy link
Member

In one recent project, the (default) standard_delete_finalizer() for an XPtr object was seen to not get called regularly, at least not in all settings leading to some complains from e.g. valgrind under full instrumentation.

The alternative, in that case, was to set explicit calls to R_RegisterCFinalizerEx() directly which we could easily add as a new alternate signature to XPtr too.

@github-actions
Copy link

This issue is stale (365 days without activity) and will be closed in 31 days unless new activity is seen. Please feel free to re-open it is still a concern, possibly with additional data.

@eddelbuettel
Copy link
Member Author

@Enchufa2 This was the issue I opened way back when. Maybe if we find some time ...

@Enchufa2
Copy link
Member

Yes, it's in the backlog. ;)

@Enchufa2
Copy link
Member

Unless I'm missing something, the workaround you implemented in TileDB is doing exactly the same as the standard delete finalizer, except that you are setting onexit=TRUE, and the standard delete finalizer disables onexit by default. You were probably running into the same issue as Jeroen in #655. Then #656 added finalizeOnExit to the template signature (disabled by default). So, again, unless I'm missing something, you just needed something like this instead of the current workaround.

@eddelbuettel
Copy link
Member Author

eddelbuettel commented Sep 28, 2021

Right on but I am also fairly certain I tried that route (setting finalizeOnExit=TRUE in my ctor calls) before going the explicit way.

Byt maybe my usecase was wrong and/or I messed another setting up. Oddly, xml2 does not seem to use it either.

@Enchufa2
Copy link
Member

Enchufa2 commented Sep 28, 2021

I mentioned that we had a similar issue in the units package. We solved it in this way.

@eddelbuettel
Copy link
Member Author

Maybe xml2 and units had it easier because there was only one C data structure each to free?

@Enchufa2
Copy link
Member

Mmmh, I suspect there could be some issue with the order in which objects are released. In the case of units, we had to force a gc run onUnload, so that finalizers were called before freeing the underlying C data structure (see here).

But in such a case, I still don't understand why explicitly calling R_RegisterCFinalizerEx makes any difference compared to letting XPtr do it for you.

@eddelbuettel
Copy link
Member Author

My comment was more about the fact that it is nice to rely on a single ut_free. That does not map so well to my use case, sadly.

@Enchufa2
Copy link
Member

Oh, because you have multiple types. But the standard_delete_finalizer is a template, you can use that one. Or you could define a partial template specialization:

namespace Rcpp {
  template <
    typename T,
    template <class> class StoragePolicy,
    void Finalizer(T*)
  >
  class XPtr<T, StoragePolicy, Finalizer, true> {};
}

Or, if you want to be able to use XPtr also with the default finalizeOnExit=false, something like this:

template <
  typename T,
  template <class> class StoragePolicy = PreserveStorage,
  void Finalizer(T*) = standard_delete_finalizer<T>
>
using XPtr2 = XPtr<T, StoragePolicy, Finalizer, true>;

@Enchufa2
Copy link
Member

It seems that partial template specialization may not work in this case. The alias template (since C++11) is definitely the way to go:

#include <iostream>

namespace test {
    template <typename A, bool B=false>
    class TestFalse {
    public:
        TestFalse() {
            std::cout << B << std::endl;
        }
    };
}

using namespace test;

template <typename A>
using TestTrue = TestFalse<A, true>;

int main() {
    TestFalse<int> x;
    TestTrue<int> y;
    return 0;
}

gives

0
1

eddelbuettel added a commit that referenced this issue Oct 1, 2021
Address #1108: enable finalizeOnExit via RCPP_USE_FINALIZE_ON_EXIT
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants