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.

Sign up for GitHub

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

Jump to bottom

Move project files into src/omnicore #16

Merged
merged 3 commits into from
May 20, 2015
Merged

Move project files into src/omnicore #16

merged 3 commits into from
May 20, 2015

Conversation

dexX7
Copy link
Member

@dexX7 dexX7 commented Apr 13, 2015

As result of the previous discussion, this PR moves the Omni Core files into src/omnicore and src/omnicore/test, without mastercore_ or omnicore_ file prefix.

Additional grouping within the subdir would probably make sense, and in the longer run I could imagine to split files and have less classes per file, for example in the case of the traditional DEx and meta DEx, or to have a dedicated file solely for transaction decoding, and another one for the actual logic.

I wouldn't prioritize this, but it should visualize the changes and the file structure:

The QT files should probably live in src/omnicore/qt, but I'm not 100 % sure at this point, how well this can be integrated.

Alternatively, it could also be possible to move the files early, then integrate #13, but leave all UI and QT files in src/qt, to reduce the impact, and address it later. Nevertheless it would be required to update the mastercore_ references, and maybe resolve other conflicts.

While I could convert #13, I assume getting this in significantly affects the UI development, and unless @zathras-crypto thinks it would absolutely be no issue, I suggest to just move the discussion from #15 to this place, and postpone the actual move, given that it's only "nice to have", but without immediat gain.

@dexX7 dexX7 mentioned this pull request Apr 13, 2015
Closed
@zathras-crypto
Copy link

Sorry, must have missed commenting on this - I support the idea - re the UI I don't think it would be too much of a hardship to fix that up to respect the new naming convention :)

@dexX7
Copy link
Member Author

dexX7 commented Apr 24, 2015

I'd like to postpone this PR until #13 was merged, and ideally mastercore_dex was split into mastercore_dex and mastercore_mdex.

To resolve the conflicts here, I have to go though the steps anyway, which doesn't take long, but it would break again, once either one of the above takes place, thus "on hold".

@dexX7 dexX7 force-pushed the oc-0.10-project-move-subdir branch from add1307 to b127e7f Compare May 11, 2015 17:33
@dexX7
Copy link
Member Author

dexX7 commented May 11, 2015

Updated, but I tend to enqueue this after the UI integration, given the conflicts it introduces.

@dexX7 dexX7 force-pushed the oc-0.10-project-move-subdir branch from b127e7f to 42a5024 Compare May 16, 2015 22:29
dexX7 added 2 commits May 20, 2015 05:52
@dexX7
Include Omni Core via seperate Makefiles
3cfb5dc 
Provides greater seperation and clarity.
@dexX7
Update header include guards and test suite names
06369cb 
Header files shall use OMNICORE_FILENAME_H.

Test suites shall be named omnicore_testsuite_tests.
@dexX7 dexX7 force-pushed the oc-0.10-project-move-subdir branch 2 times, most recently from d75cdca to 251f09d Compare May 20, 2015 04:06
@dexX7
Copy link
Member Author

dexX7 commented May 20, 2015

@zathras-crypto: given that the UI branch was merged, it's probably a good time to merge this one too. Do you agree?

@dexX7 dexX7 force-pushed the oc-0.10-project-move-subdir branch from 251f09d to dc7c594 Compare May 20, 2015 04:30
@zathras-crypto
Copy link

given that the UI branch was merged, it's probably a good time to merge this one too. Do you agree?

Sorry bud I've been slack on this one - I haven't actually had a chance to look at what's required to move the UI files to /src/omnicore/qt yet

@dexX7
Copy link
Member Author

dexX7 commented May 20, 2015

Currently only the src/mastercore*, src/omnicore* and src/test/mastercore* files are moved, while the the QT files are still in src/qt/, which is fine, I guess, and it could be done later, but I haven't really made up my mind how the secondary files (e.g. the images etc.) could fit into this nicely.

I mentioned the UI PR, because it touched many files, and most of them were affected by the src/ -> src/omnicore/ move.

@zathras-crypto
Copy link

Ah OK then - no objections here :)

@zathras-crypto
Copy link

Sorry, accidental close there :(

@dexX7
Copy link
Member Author

dexX7 commented May 20, 2015

Haha, no worries. :)

@dexX7 dexX7 merged commit dc7c594 into OmniLayer:omnicore-0.0.10 May 20, 2015
dexX7 added a commit that referenced this pull request May 20, 2015
@dexX7
Merge pull request #16
80596c4 
dc7c594 Move project files into src/omnicore/ and src/omnicore/test/ (dexX7)
06369cb Update header include guards and test suite names (dexX7)
3cfb5dc Include Omni Core via seperate Makefiles (dexX7)
@dexX7 dexX7 modified the milestone: 0.0.10.0 Dec 30, 2015
bvbfan pushed a commit to bvbfan/omnicore that referenced this pull request Jun 6, 2023
@fanquake
Merge bitcoin#25077: Fix chain tip data race and corrupt rest response
a75b779 
fac04cb refactor: Add lock annotations to Active* methods (MacroFake)
fac15ff Fix logical race in rest_getutxos (MacroFake)
fa97a52 Fix UB/data-race in RPCNotifyBlockChange (MacroFake)
fa530bc Add ChainstateManager::GetMutex(), an alias for ::cs_main (MacroFake)

Pull request description:

  This fixes two issues:

  * A data race in `ActiveChain`, which returns a reference to the chain (a `std::vector`), which is not thread safe. See also below traceback.
  * A corrupt rest response, which returns a blockheight and blockhash, which are unrelated to each other and to the result, as the chain might advance between each call without cs_main held.

  The issues are fixed by taking cs_main and holding it for the required time.

  ```
  ==================
  WARNING: ThreadSanitizer: data race (pid=32335)
    Write of size 8 at 0x7b3c000008f0 by thread T22 (mutexes: write M131626, write M151, write M131553):
      #0 std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&) /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 (bitcoind+0x501239)
      OmniLayer#1 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__swap_out_circular_buffer(std::__1::__split_buffer<CBlockIndex*, std::__1::allocator<CBlockIndex*>&>&) /usr/lib/llvm-13/bin/../include/c++/v1/vector:977:5 (bitcoind+0x501239)
      OmniLayer#2 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::__append(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:1117:9 (bitcoind+0x501239)
      OmniLayer#3 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::resize(unsigned long) /usr/lib/llvm-13/bin/../include/c++/v1/vector:2046:15 (bitcoind+0x4ffe29)
      OmniLayer#4 CChain::SetTip(CBlockIndex*) src/chain.cpp:19:12 (bitcoind+0x4ffe29)
      OmniLayer#5 CChainState::ConnectTip(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, ConnectTrace&, DisconnectedBlockTransactions&) src/validation.cpp:2748:13 (bitcoind+0x475d00)
      OmniLayer#6 CChainState::ActivateBestChainStep(BlockValidationState&, CBlockIndex*, std::__1::shared_ptr<CBlock const> const&, bool&, ConnectTrace&) src/validation.cpp:2884:18 (bitcoind+0x47739e)
      OmniLayer#7 CChainState::ActivateBestChain(BlockValidationState&, std::__1::shared_ptr<CBlock const>) src/validation.cpp:3011:22 (bitcoind+0x477baf)
      OmniLayer#8 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74)
      OmniLayer#9 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e)
      OmniLayer#10 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e)
      OmniLayer#11 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e)
      OmniLayer#12 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e)
      OmniLayer#13 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e)
      OmniLayer#14 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f)
      OmniLayer#15 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f)
      OmniLayer#16 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f)
      OmniLayer#17 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a)
      OmniLayer#18 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a)
      OmniLayer#19 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a)
    Previous read of size 8 at 0x7b3c000008f0 by main thread:
      #0 std::__1::vector<CBlockIndex*, std::__1::allocator<CBlockIndex*> >::size() const /usr/lib/llvm-13/bin/../include/c++/v1/vector:680:61 (bitcoind+0x15179d)
      OmniLayer#1 CChain::Tip() const src/./chain.h:449:23 (bitcoind+0x15179d)
      OmniLayer#2 ChainstateManager::ActiveTip() const src/./validation.h:927:59 (bitcoind+0x15179d)
      OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1841:35 (bitcoind+0x15179d)
      OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Location is heap block of size 232 at 0x7b3c00000870 allocated by main thread:
      #0 operator new(unsigned long) <null> (bitcoind+0x132668)
      OmniLayer#1 ChainstateManager::InitializeChainstate(CTxMemPool*, std::__1::optional<uint256> const&) src/validation.cpp:4851:21 (bitcoind+0x48e26b)
      OmniLayer#2 node::LoadChainstate(bool, ChainstateManager&, CTxMemPool*, bool, Consensus::Params const&, bool, long, long, long, bool, bool, std::__1::function<bool ()>, std::__1::function<void ()>) src/node/chainstate.cpp:31:14 (bitcoind+0x24de07)
      OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1438:32 (bitcoind+0x14e994)
      OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Mutex M131626 (0x7b3c00000898) created at:
      #0 pthread_mutex_lock <null> (bitcoind+0xda898)
      OmniLayer#1 std::__1::mutex::lock() <null> (libc++.so.1+0x49f35)
      OmniLayer#2 node::ThreadImport(ChainstateManager&, std::__1::vector<fs::path, std::__1::allocator<fs::path> >, ArgsManager const&) src/node/blockstorage.cpp:883:30 (bitcoind+0x23cd74)
      OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7::operator()() const src/init.cpp:1657:9 (bitcoind+0x15863e)
      OmniLayer#4 decltype(static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(fp)()) std::__1::__invoke<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x15863e)
      OmniLayer#5 void std::__1::__invoke_void_return_wrapper<void, true>::__call<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&>(AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&) /usr/lib/llvm-13/bin/../include/c++/v1/__functional/invoke.h:61:9 (bitcoind+0x15863e)
      OmniLayer#6 std::__1::__function::__alloc_func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:171:16 (bitcoind+0x15863e)
      OmniLayer#7 std::__1::__function::__func<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, std::__1::allocator<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>, void ()>::operator()() /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:345:12 (bitcoind+0x15863e)
      OmniLayer#8 std::__1::__function::__value_func<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:498:16 (bitcoind+0x88891f)
      OmniLayer#9 std::__1::function<void ()>::operator()() const /usr/lib/llvm-13/bin/../include/c++/v1/__functional/function.h:1175:12 (bitcoind+0x88891f)
      OmniLayer#10 util::TraceThread(char const*, std::__1::function<void ()>) src/util/thread.cpp:18:9 (bitcoind+0x88891f)
      OmniLayer#11 decltype(static_cast<void (*>(fp)(static_cast<char const*>(fp0), static_cast<AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(fp0))) std::__1::__invoke<void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/type_traits:3918:1 (bitcoind+0x157e6a)
      OmniLayer#12 void std::__1::__thread_execute<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, 2ul, 3ul>(std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7>&, std::__1::__tuple_indices<2ul, 3ul>) /usr/lib/llvm-13/bin/../include/c++/v1/thread:280:5 (bitcoind+0x157e6a)
      OmniLayer#13 void* std::__1::__thread_proxy<std::__1::tuple<std::__1::unique_ptr<std::__1::__thread_struct, std::__1::default_delete<std::__1::__thread_struct> >, void (*)(char const*, std::__1::function<void ()>), char const*, AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7> >(void*) /usr/lib/llvm-13/bin/../include/c++/v1/thread:291:5 (bitcoind+0x157e6a)
    Mutex M151 (0x55aacb8ea030) created at:
      #0 pthread_mutex_init <null> (bitcoind+0xbed2f)
      OmniLayer#1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3)
      OmniLayer#2 __libc_start_main <null> (libc.so.6+0x29eba)
    Mutex M131553 (0x7b4c000042e0) created at:
      #0 pthread_mutex_init <null> (bitcoind+0xbed2f)
      OmniLayer#1 std::__1::recursive_mutex::recursive_mutex() <null> (libc++.so.1+0x49fb3)
      OmniLayer#2 std::__1::__unique_if<CTxMemPool>::__unique_single std::__1::make_unique<CTxMemPool, CBlockPolicyEstimator*, int const&>(CBlockPolicyEstimator*&&, int const&) /usr/lib/llvm-13/bin/../include/c++/v1/__memory/unique_ptr.h:728:32 (bitcoind+0x15c81d)
      OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1426:24 (bitcoind+0x14e7b4)
      OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
    Thread T22 'b-loadblk' (tid=32370, running) created by main thread at:
      #0 pthread_create <null> (bitcoind+0xbd5bd)
      OmniLayer#1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:443:10 (bitcoind+0x155e06)
      OmniLayer#2 std::__1::thread::thread<void (*)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7, void>(void (*&&)(char const*, std::__1::function<void ()>), char const (&) [8], AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_7&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (bitcoind+0x155e06)
      OmniLayer#3 AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*) src/init.cpp:1656:29 (bitcoind+0x150164)
      OmniLayer#4 AppInit(node::NodeContext&, int, char**) src/bitcoind.cpp:231:43 (bitcoind+0x133fd2)
      OmniLayer#5 main src/bitcoind.cpp:275:13 (bitcoind+0x133fd2)
  SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/__utility/swap.h:39:7 in std::__1::enable_if<(is_move_constructible<CBlockIndex**>::value) && (is_move_assignable<CBlockIndex**>::value), void>::type std::__1::swap<CBlockIndex**>(CBlockIndex**&, CBlockIndex**&)
  ==================
  ```

  From https://cirrus-ci.com/task/5612886578954240?logs=ci#L4868

ACKs for top commit:
  achow101:
    re-ACK fac04cb
  theStack:
    Code-review ACK fac04cb

Tree-SHA512: 9d619f99ff6373874c7ffe1db20674575605646b4b54b692fb54515a4a49f110a770026d7320ed6dfeaa7976be4cd89e93f821acdbf22c7662bd1c5be0cedcd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
request for comments status: ready
Projects
None yet
Milestone
0.0.10.0
Development

Successfully merging this pull request may close these issues.
2 participants
@dexX7 @zathras-crypto