Assertion failed after run first time Windows 10

[nanomo]

Step to reproduce:

• install from exe http://omnichest.info/files/omnicore-0.0.9.99-dev-win64-setup.exe
• add direct link and --testnet param to shotcut
• start omni
• after ask to update or edit some config file will show:
  

tried a second time to start and the error its gone.

[zathras-crypto]

Hi @nanomo thanks very much for testing, and for the bug report :)

I spun up a Win 10 VM and tested the current preview (based on b50c70e) in the scenario described - can confirm the bug.

Repeat tests can be performed by deleting %appdata%\Bitcoin, 4 out of 5 test startups displayed this symptom (assert in the LockedPageManagerBase class) after writing the txindex config modification out to disk.

What's really interesting is I spun up a Win 7 VM and tested the current preview, and confirm the bug is there as well. I'm not sure how I haven't seen this before since it's so easily reproducible (though my Windows testing of 0.0.9.99 has been quite limited).

The issue seems to be with writing the config, 0 of 5 tests triggered the assert failure when not opting to amend the configuration, and 0 of 5 tests triggered the assert once the config file had been updated.

Our latest build (based on 01d49a6) also suffers from the same issue.

It seems to be the calling of StartShutdown() here https://github.com/OmniLayer/omnicore/blob/omnicore-0.0.10/src/init.cpp#L1143 during init (which we do to force the user to reload Omni Core with the new config) - perhaps that's not a good enough approach given the wallet (and various other bits) are not yet init when we call for a shutdown.

We could look at the order of events here, or even perhaps return InitError with the "configuration updated message" instead, same end result with potentially sidestepping any issues relating to a premature shutdown call - doesn't seem as clean somehow though because it's technically not an error hehe :)

[dexX7]

Thanks for the report @nanomo, this is very helpful!

I've seen this error a few times before, but I'm not 100 % sure when. I think when I forced a shutdown.

So far I was unable to reproduce this issue with b50c70e on Win 10, but if it's some kind of shutdown order issue, I'm not surprised.

@zathras-crypto: you mentioned StartShutdown(): this part was recently removed and updated via f86a1be ("Show information icon, not error icon, when asking for txindex flag").

Since InitError() creates a message box and just returns, we should be able to replace the StartShutdown() with return false and have the same effect (while keeping the icons).

[zathras-crypto]

So far I was unable to reproduce this issue with b50c70e on Win 10, but if it's some kind of shutdown order issue, I'm not surprised.

That's interesting... I was able to reproduce quite regularly on Win 7 & Win 10 by deleting %appdata%\bitcoin - I wonder why the disparity?

@zathras-crypto: you mentioned StartShutdown():

Ahh thank you for that commit hash :) OK so I created two sets of windows binaries:
f146bfc
f86a1be

One set based on the commit you mentioned and one set based on the preceding commit (which fixes another crash on exit after changing txindex hehe). I tested them out on a Win VM and can confirm that there were no crashes in the f146bfc build and there were crashes in the f86a1be build (when we started using StartShutdown) so I think it's a pretty safe bet that's the culprit and we should swap it out.

I merged in the fix here 8eb3afa and ran another set of binaries based on this new tip, tested them out and can no longer reproduce the issue so should be solved.

[dexX7]

Thanks for testing this @zathras-crypto!

I wonder why the disparity?

This could be caused by different system specs, and especially the initialization and shutdown order of variables is not necessarily deterministic.

You mentioned:

4 out of 5 test startups displayed this symptom

[dexX7]

This issue was resolved via #221.