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

boost::di::extension::shared_config doesn't work correctly with MSVC release build #512

Closed
JulZimmermann opened this issue Mar 11, 2021 · 7 comments

Comments

@JulZimmermann
Copy link

Hi, first off: Thank you for this amazing library!

Unfortunately, I found a bug with boost::di::extension::shared_config when using the MSVC compiler in release mode.

Take this minimal example:

#include <iostream>

#include "boost/di.hpp"
#include "boost/di/extension/scopes/shared.hpp"

class Bar {
public:
    int getI() const { return i; }

private:
    int i = 2;
};

class Foo {
public:
    Foo(std::shared_ptr<Bar> bar) : bar(std::move(bar)) {}
    std::shared_ptr<Bar> getBar() {  return bar; }

private:
    std::shared_ptr<Bar> bar;
};

class IBaz {
public:
    virtual ~IBaz() = default;
};

class Baz : public IBaz {
};

int main() {
    auto injector = boost::di::make_injector<boost::di::extension::shared_config>();

    auto baz = injector.create<std::shared_ptr<Baz>>();
    auto foo = injector.create<std::unique_ptr<Foo>>();

    std::cout << std::to_string(foo->getBar()->getI()) << std::endl;
}

This code runs fine with GCC in debug and release mode, as well as with MSVC in Debug mode and this program will print 2.
A release build created with the MSVC compiler will print a random number because of some lifetime problem.

Some things I observed:

  • Changing class Baz : public IBaz to class Baz (not implementing the interface) will cause the expected behavior
  • Changing boost::di::make_injector<boost::di::extension::shared_config> to boost::di::make_injector will also cause the expected behavior

Expected Behavior

Output: 2

Actual Behavior

Undefined Behavior

Specifications

  • Version: v1.2.0 as well as the current master
  • Platform: Windows with MSVC 19.28.29336.0
@kanstantsin-chernik
Copy link
Collaborator

Weird bug...
You can try a workaround to bind types explicitly:

  auto injector = boost::di::make_injector<boost::di::extension::shared_config>(
      boost::di::bind<Bar>().in(boost::di::extension::shared)
  );

@kanstantsin-chernik
Copy link
Collaborator

The problem is these types have same type_ids:

type_id: 140700154865104 name: "struct Baz"
type_id: 140700154865104 name: "struct Bar"

@kanstantsin-chernik
Copy link
Collaborator

kanstantsin-chernik commented Mar 11, 2021

you can use this temporary instead of shared.hpp extension:

#pragma once

#include <memory>
#include <typeindex>
#include <unordered_map>
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
#include <mutex>
#endif

#include "boost/di.hpp"
#include <string>

BOOST_DI_NAMESPACE_BEGIN
namespace extension {

namespace detail {
class shared {
 public:
  template <class, class T>
  class scope {
   public:
    scope() noexcept = default;

#if !defined(BOOST_DI_NOT_THREAD_SAFE)
    //<<lock mutex so that move will be synchronized>>
    explicit scope(scope&& other) noexcept : scope(std::move(other), std::lock_guard<std::mutex>(other.mutex_)) {}
    //<<synchronized move constructor>>
    scope(scope&& other, const std::lock_guard<std::mutex>&) noexcept : object_(std::move(other.object_)) {}
#endif

    template <class T_, class>
    using is_referable = typename wrappers::shared<shared, T>::template is_referable<T_>;

    template <class, class, class TProvider>
    static auto try_create(const TProvider& provider)
        -> decltype(wrappers::shared<shared, T>{std::shared_ptr<T>{provider.get()}});

    /**
     * `in(shared)` version
     */
    template <class, class, class TProvider>
    wrappers::shared<shared, T> create(const TProvider& provider) & {
      if (!object_) {
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
        std::lock_guard<std::mutex> lock(mutex_);
        if (!object_)
#endif
          object_ = std::shared_ptr<T>{provider.get()};
      }
      return wrappers::shared<shared, T>{object_};
    }

    /**
     * Deduce scope version
     */
    template <class, class, class TProvider>
    wrappers::shared<shared, T> create(const TProvider& provider) && {
      auto& object = provider.cfg().template data<T>();
      if (!object) {
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
        std::lock_guard<std::mutex> lock(mutex_);
        if (!object)
#endif
          object = std::shared_ptr<T>{provider.get()};
      }
      return wrappers::shared<shared, T>{std::static_pointer_cast<T>(object)};
    }

   private:
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
    std::mutex mutex_;
#endif
    std::shared_ptr<T> object_;  /// used by `in(shared)`, otherwise destroyed immediately
  };
};
}  // namespace detail

static constexpr detail::shared shared{};

class shared_config : public di::config {
  template <class T>
  struct type {
    static void id() {}
  };
  template <class T>
  static auto type_id() {
    return reinterpret_cast<std::size_t>(&type<T>::id);
  }

 public:
  template <class T>
  struct scope_traits {
    using type = typename di::config::scope_traits<T>::type;
  };

  template <class T>
  struct scope_traits<T&> {
    using type = detail::shared;
  };

  template <class T>
  struct scope_traits<std::shared_ptr<T>> {
    using type = detail::shared;
  };

  template <class T>
  struct scope_traits<boost::shared_ptr<T>> {
    using type = detail::shared;
  };

  template <class T>
  struct scope_traits<std::weak_ptr<T>> {
    using type = detail::shared;
  };

  template <class T>
  auto& data() {
    return data_[typeid(T).name()];
  }

 private:
  std::unordered_map<std::string, std::shared_ptr<void>> data_{};
};

}  // namespace extension
BOOST_DI_NAMESPACE_END

@kanstantsin-chernik
Copy link
Collaborator

Hey @krzysztof-jusiak. Seems like MS reuses typeid for similar types. We cannot rely on id() any more.

@krzysztof-jusiak
Copy link
Collaborator

hmm, weird @kanstantsin-chernik. but we can always switch to simply use typeinfo/type_index 🤔

@kanstantsin-chernik
Copy link
Collaborator

@krzysztof-jusiak Great idea! It actually works. Let me send a PR

@JulZimmermann
Copy link
Author

Thank you so much for fixing this so quickly. I can confirm the problem is gone now,

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

No branches or pull requests

3 participants