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

sherwood_v8_block::empty_block() oddities #14

Open
markpapadakis opened this issue Jun 4, 2018 · 1 comment
Open

sherwood_v8_block::empty_block() oddities #14

markpapadakis opened this issue Jun 4, 2018 · 1 comment

Comments

@markpapadakis
Copy link

I noticed that sometimes sherwood_v8_table::emplace() would just fail even if the instance was empty(no elements). It turns out, that this happens because sometimes sherwood_v8_table::empty_block() would return a different pointer than you would get if you, say, printed that in the constructor, and if that 'd happen, sherwood_v8_table::emplace() would end up invoking rehash(), and that would, in turn, invoke deallocate_data() and (begin == BlockType::empty_block()) evaluate to false and so it 'd try to deallocate memory that was never allocated.

t made no sense, and, while I did put some time into trying to understand it, nothing immediately obvious was found, and it was only reproducible under specific conditions. I suspect it manifests when dealing with dlls/dsos. One quick fix involved changing the definition to:

static inline sherwood_v8_block *empty_block() {
        static std::unique_ptr<int8_t[]> data([] {
                auto ptr = new int8_t[BlockSize];

                std::fill(ptr, ptr + BlockSize, sherwood_v8_constants<>::magic_for_empty);
                return ptr;
        }());

        return reinterpret_cast<sherwood_v8_block *>(data.get());
}
@cebtenzzre
Copy link

cebtenzzre commented Jan 25, 2019

I'm fairly certain the C++ standard guarantees that the existing code is correct. Dynamic linkage alone shouldn't affect this unless your compiler or linker has bugs.
I can only imagine three ways this could break:

  • If you're doing something weird with threading that makes all variables with static storage duration thread-local (I don't know if that's even possible), then if empty_block is called from two different threads, it may result in two different pointers.
  • If sherwood_v8_block is defined differently in two different translation units (e.g. defining certain macros before including the header, or using different versions of the header), it could result in two separate versions of it, breaking ODR.
  • If you're interpreting an instance of the class in two different ways using casts, you may be calling two separate versions of this function.

If changing the definition as you said actually fixes this, then I think that only the first case could be true.

EDIT: See #26. Depending on how the standard is interpreted, shared objects may not be considered translation units as per the standard's usual definition. Linux has the safer and more conforming behavior, Windows may not. This StackOverflow question explains this in more detail.

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

2 participants