Runtime assertion on OS X

[leto]

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]

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]

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

[leto]

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

[leto]

@DeckerSU do you have any ideas here?

[tonymorony]

@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]

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

[DeckerSU]

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.

[jl777]

thanks! i will add a maxsize parameter to Split

[tonymorony]

Was fixed by this commit: 152c86c

[miketout]

Thanks all! I'll mirror the change to make this more general in Verus as well!

…

On Fri, Jul 12, 2019 at 5:51 PM DeckerSU ***@***.***> wrote: 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 <https://github.com/jl777/komodo/blob/master/CONTRIBUTING.md>, offered by @ca333 <https://github.com/ca333>. Even for such "small changes", such as ASSETCHAINS_MAX_ERAS change. I found the root of issue: Split <https://github.com/jl777/komodo/blame/beta/src/util.cpp#L417> function was written by @miketout <https://github.com/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 <https://github.com/jl777/komodo/blob/8c4362ed0e8190f025fafefcf4525383fcbe96c8/src/komodo_utils.h#L1761> : 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 https://github.com/jl777/komodo/blob/8c4362ed0e8190f025fafefcf4525383fcbe96c8/src/komodo_utils.h#L1761 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. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1606?email_source=notifications&email_token=AFTGWKBLFRLP576OG4UEG5LP7ERKPA5CNFSM4IBY7GEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ3FV6Y#issuecomment-511073019>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFTGWKGLMHOM3SQW7UG5L3TP7ERKPANCNFSM4IBY7GEA> .