jl777/komodo

Runtime assertion on OS X

Closed this issue · 10 comments

leto commented

I am seeing this on the latest jl777 branch and latest dev branch (commit 49cacbf) :

$ ./komodod
call komodo_args.(./komodod) NOTARY_PUBKEY.()
initialized  at 1562887488
Assertion failed: (px != 0), function operator*, file /Users/jonathanleto/git/komodo/depends/x86_64-apple-darwin17.5.0/share/../include/boost/smart_ptr/shared_ptr.hpp, line 728.

Full backtrace is here, but it's not very useful:

https://gist.github.com/leto/a8088234a7c575f963b8063db87c3183

From what I can see, something in Boost internals is not happy and the seemingly innocuous line of code uiInterface.InitMessage(_("Verifying wallet...")); causes that assertion inside boost shared_ptr header file

jl777 commented

the only recent change to startup was deckers speedup by skipping chainpower

main.cpp lines 150 or so: struct CBlockIndexWorkComparator
can you try building on osx where it always does the ASSETCHAINS_LWMAPOS path and also remove the const:
bool operator()(const CBlockIndex *pa, const CBlockIndex *pb) ->
bool operator()(CBlockIndex *pa, const CBlockIndex *pb) const {

tonymorony commented

by exception method I've found that this 7362634 change causing this OSX specific startup crash

leto commented

@tonymorony can you explain more? Did you use git bisect to find that commit?

leto commented

@DeckerSU do you have any ideas here?

tonymorony commented

@leto master starts fine on OSX - by diff between master and beta/dev I found exact commit on which things started to fail by multiply builds with dividing scope twice on each attempt.

Now if you change 7 to 3 here https://github.com/jl777/komodo/blob/dev/src/komodo_defs.h#L21 and rebuild - daemon should start. Could you please try?

leto commented

@tonymorony yes, things work fine with a max era value of 3, I am able to sync

DeckerSU commented

That's why i'm always told that every PR should be verified + tested (MacOS with gcc-8 is a very good case to find an non-obvious errors) and approved first (!) and we should always follow cotribution guide, offered by @ca333. Even for such "small changes", such as ASSETCHAINS_MAX_ERAS change. I found the root of issue:

Split function was written by @miketout for splitting numeric params related to VRSC eras. But later we start using it everywhere, even for splitting non-eras related AC params despite the fact that it has following code inside:

for ( i = numVals; i < ASSETCHAINS_MAX_ERAS; i++ )
    {
        outVals[i] = nLast;
    }

So, it was intended to use only with eras related params.

For example here:
Split(GetArg("-ac_nk",""), ASSETCHAINS_NK, 0);
And ASSETCHAINS_NK declared as array with only two elements (N,K).

Of course, mindless change of ASSETCHAINS_MAX_ERAS from 3 to 7 will cause various non-obvious errors in various places. Bcz every Split(GetArg(..,..), ...) depends on it.

To make sure that i'm correct in above, just comment this line

komodo/src/komodo_utils.h

Line 1761 in 8c4362e

Split(GetArg("-ac_nk",""), ASSETCHAINS_NK, 0);
and error above will gone.

So, we need to decide, if we really want to change ASSETCHAINS_MAX_ERAS from 3 to 7, we should check every Split call to be sure that all will be correct. May be good idea is to add nMaxSplitSize param to it, like this:

void Split(const std::string& strVal, uint64_t *outVals, const uint64_t nDefault, const uint64_t nMaxSplitSize = ASSETCHAINS_MAX_ERAS)

and change this:

for ( i = numVals; i < ASSETCHAINS_MAX_ERAS; i++ )

to this:

for ( i = numVals; i < nMaxSplitSize; i++ )

Inside Split. Then, check every Split call in sources and add nMaxSplitSize param where needed, like:
Split(GetArg("-ac_nk",""), ASSETCHAINS_NK, 0, 3);

So, as a result we have following ways to solve it:

  1. Revert ASSETCHAINS_MAX_ERAS to 3.
  2. Modify Split function to pay attention to outVals size by creating nSize argument, and check every Split call to specify real array size.
  • 👍1
jl777 commented

thanks! i will add a maxsize parameter to Split

tonymorony commented

Was fixed by this commit: 152c86c

miketout commented
  • 👎1