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

lock/mutex issues (client crashing), do you have it? #564

Closed
WagerrTor opened this issue Mar 12, 2018 · 18 comments
Closed

lock/mutex issues (client crashing), do you have it? #564

WagerrTor opened this issue Mar 12, 2018 · 18 comments

Comments

@WagerrTor
Copy link

WagerrTor commented Mar 12, 2018

We had discussion about lock issues. Some pivx holders reported that their wallets crashed and I do not find this issue on your list. We might have same issue. One of our devs/contributors has posted some suggestion, you can look up here. He supposes that during zerocoin implementation you might not have adapted mutexes properly.

We are middle in this discussion and are proposing currently changes but since couple of days there were several people reporting their pivx clients crashed, I have no reports about our clients but if this is issue for you, then it is for us probably too.

@WagerrTor
Copy link
Author

You can find this issue on our git and here is the commit which we currently test.

@ethanedison
Copy link

Issue is fixed by following some recent posts of cleaning up some of the wallet exe commands.

If need more info on how I fixed ( i am not a developer) I can dig into this and send you the steps on how I solved the crashing issues.

@Kokary
Copy link

Kokary commented Mar 12, 2018

Not sure that's the same issue; we noticed a potential missing lock on cs_main during staking, manifesting itself in the increasingly unlikely scenario that a stake is found at the same time when an incoming block is validated, triggering an assert error with such debug output:
Error:'Assertion failed: lock cs_main not held in wallet.cpp:4261; locks held:'

Minting input selection code added with the ZeroCoin code doesn't have the mutex locks that Bitcoin added to similar 'classical' code. Could of course be that you're already addressing this when moving to ZeroMint.

@Mrs-X
Copy link

Mrs-X commented Mar 12, 2018

@WagerrTor Thanks for the feedback and information, it's very appreciated 👍

Did you or one of your users find a reliable way to reproduce those crashes? Which OS?

Edit: I ask because I have several wallets on both main- and testnet staking for months now without any issues.

@Kokary
Copy link

Kokary commented Mar 12, 2018

Producing many stakes on a low CPU machine should work ;) (It happened when setting up testnet.) Or have a lot of bad luck.. Could be an edge case on large networks.

@Mrs-X
Copy link

Mrs-X commented Mar 12, 2018

Hmh, I don't have any low CPU computers, but I've tested staking on a (linux-) machine with a load of > 80 which was swapping like hell, means the CPU didn't get much cycles to run.
But I'll ask around if one of the other devs can reproduce it.

@WagerrTor
Copy link
Author

As I said, sadly we did not have real opportunity to look very deep into that and are currently testing what @Kokary suggested. It is very fresh issue and we had to rub our heads first to see if this is at all an issue and if it is, does it affect us too. So far, after 1 night I am reported that the error does not happen anymore.

It happend like @Kokary explained on non main network. I was not able to reproduce it as I did not have any as addition, like I said, it is very fresh issue for us where it came up not by our own wallets directly but by users reporting their pivx wallet crashes (hope they will create issues here, otherwise you probably will have not enough input for that).

@ethanedison
Copy link

Here is what I did to fix my issue:

  1. go to PIVX data directory :

  2. deleted these files:

blocks, chain state, .lock, budget.dat, db.log, debug.log, fee_estimates.dat, mncache.dat, peers.dat,

  1. deleted the folders called: zerocoin & sporks

4)) Relaunched wallet and sync with network after deleted files

@WagerrTor
Copy link
Author

@ethanedison thanks for suggestion. Your suggestion is actually more a workaround which can be applied when your wallet crashes, you could achieve the same by simply resyncing too, which would not require data folder deletion. Point is, wallet should not crash and we try to find out the reason why it crashed in first place which requires additional steps and we think we found it, at least having no crashes/errors looks like if it had some positive effect if not a fix.

@ethanedison
Copy link

ethanedison commented Mar 18, 2018 via email

@Mrs-X
Copy link

Mrs-X commented Mar 18, 2018

@ethanedison Add enablezeromint=0 to your pivx.conf and restart the wallet.

(yes, the next release will have this setting available from the GUI)

@ethanedison
Copy link

ethanedison commented Mar 20, 2018 via email

@WagerrTor
Copy link
Author

WagerrTor commented Mar 21, 2018

@ethanedison my best advice to you is, trust that in 9 of 10 cases it is always users fault. I would think in your case it is the same. Before you use any software, you should probably read first the documentation about it and maybe running help in your debug console of your gui or daemon is also not bad idea, it does show description of different commands. In case of enablezeromint, you would have found it if you would have tried to run the daemon with --help switch. You might want to know about those commands too and yes, you can place them in your config too:

Zerocoin options:

  -enablezeromint=<n>
       Enable automatic Zerocoin minting (0-1, default: 1)

  -zeromintpercentage=<n>
       Percentage of automatically minted Zerocoin (10-100, default: 0)

  -preferredDenom=<n>
       Preferred Denomination for automatically minted Zerocoin
       (1/5/10/50/100/500/1000/5000), 0 for no preference. default: 0)

  -backupzpiv=<n>
       Enable automatic wallet backups triggered after each zPiv minting (0-1,
       default: 1)

What you probably would not have found out by --help is about zeromint=0. @Mrs-X probably help and description should be added for zeromint.

My suggestion to you would be to disable zeromint in general and you can do so by placing in your config: zeromint=0 or by starting your client with the same switch.

If you want to be really 100% sure, you should disable automint for gui as well to disable zeromint:

zeromint=0
enablezeromint=0

Now your wallet will not automint anymore.

I hope my reply can help you solving your problem, I would suggest you to open separte issue about it, as here it is totaly offtopic and actually has nothing to do with the issue discussed here.

@Fuzzbawls
Copy link
Collaborator

@WagerrTor we don't have a runtime option called zeromint

@rejectedpromise
Copy link

Is this still an active issue?

@Kokary
Copy link

Kokary commented May 9, 2018

I didn't dig into what was and wasn't addressed in the very nice, large update, however, at least partially the issue still persist. E.g.:

https://github.com/PIVX-Project/PIVX/blob/master/src/wallet.cpp#L2093
out.tx->IsInMainChain() calls return GetDepthInMainChainINTERNAL(pindexRet) > 0;, which has:

int CMerkleTx::GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const
{
    if (hashBlock == 0 || nIndex == -1)
        return 0;
    AssertLockHeld(cs_main);

More importantly, the same happens in CWallet::AutoCombineDust() :
https://github.com/PIVX-Project/PIVX/blob/master/src/wallet.cpp#L4099

And finally (in what I observed), void CWallet::UnlockCoin(COutPoint& output) has AssertLockHeld(cs_wallet); but is called on several locations without acquiring that lock, e.g.:

https://github.com/PIVX-Project/PIVX/blob/master/src/activemasternode.cpp#L444

I'd be happy to submit a PR for these at least, but I don't know if and how anyone else is looking at this.

@Mrs-X
Copy link

Mrs-X commented May 10, 2018

I guess when our latest release has stabilized we will have more time to look into this.

A PR would be highly appreciated of course 👍

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

8 participants
@furszy @Fuzzbawls @Mrs-X @rejectedpromise @ethanedison @WagerrTor @Kokary and others