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

Travis-CI memory reference errors in CLI Tests #2067

Closed
MichelSantos opened this issue Nov 23, 2019 · 5 comments
Closed

Travis-CI memory reference errors in CLI Tests #2067

MichelSantos opened this issue Nov 23, 2019 · 5 comments

Comments

@MichelSantos
Copy link
Contributor

MichelSantos commented Nov 23, 2019

Travis-CI has detected some memory reference problem in the CLI wallet integration tests. These errors cannot always be triggered.

  • Some Travis results indicate that the memory reference problem is connected with the construction of the fixture.

  • Other Travis results also indicate that the memory reference problem is connected with delays after app1->shutdown in the preceding unit test.

  • The memory reference errors can be imitated outside of Travis by the inclusion of the following code (which is patently ridiculous by intentionally breaking the functionality) at the beginning of the cli_fixture constructor.

std::cout << "CLI_Fixture A" << std::endl;

app1->shutdown();
fc::usleep(fc::milliseconds(1000));

std::cout << "CLI_Fixture B" << std::endl;

These combined test results suggest a timing problem where the conclusion of one unit test is interfering with the startup of the next unit test as in cli_multisig_transaction-saving_keys_wallet_test, and cli_create_htlc-cli_sign_message.

@abitmore
Copy link
Member

abitmore commented Nov 24, 2019

Duplicate of #1303 ? Also see #1626, #1856 and #1784. If it's the same issue or similar, it's has been there for quite some time, which is annoying, but imo it's not blocking anything. I'd say leave it there now. I had a plan to fix it after merged bitshares/bitshares-fc#134 (but that one is a bit stale).

@MichelSantos
Copy link
Contributor Author

Yes the nature of the same problem is the same as exposed by #1303 which is a problem of one instance of an object from a prior test interfering, through an as yet unconfirmed mechanism (although I have my guesses), with another instance in the subsequent test. The fix in #1626 attempts to fix it by allowing the environment sufficient time to shut down the prior instance of the cli_fixture members.

However the delaying action from #1626 is absent when the prior test creates an graphene::app::application directly rather than a cli_fixture. This causes a problem for two pairs of test sequences in the suite: cli_multisig_transaction-saving_keys_wallet_test, and cli_create_htlc-cli_sign_message. In both cases, the prior test directly creates an instance of graphene::app::application and the subsequent test indirectly creates an instance of graphene::app::application by instantiating a cli_fixture.

I have no objection to leaving out this PR for now because I have high confidence at this point that the problem is in the test suite and not in the underlying objects in Core. Please advise if you recommend to close this PR #2068 or to PR into another branch.

@abitmore
Copy link
Member

I didn't know you created another app object outside of cli_fixture. Approved #2068.

@abitmore
Copy link
Member

The underlying reason is described in OP of #1784.

oxarbitrage added a commit that referenced this issue Nov 26, 2019
@pmconrad
Copy link
Contributor

Hopefully fixed by #2068. Failure of merge build was due to timeout in sonar scanner.

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

3 participants