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.

Sign up for GitHub

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

Jump to bottom

Wallet FFI not shutting down coms #5984

Closed
SWvheerden opened this issue Nov 24, 2023 · 7 comments
Closed

Wallet FFI not shutting down coms #5984

SWvheerden opened this issue Nov 24, 2023 · 7 comments
Assignees
@hansieodendaal
Labels
release-blocker Something that needs to be fixed before a release can be made

Comments

@SWvheerden
Copy link
Collaborator

After wallet_destory is called, the wallet should shutdown, and mostly does.
How ever the comms is not shutting down and keeps trying to connect in the backend.
@SWvheerden SWvheerden added the P-high-risk Process - High risk label Nov 24, 2023
@SWvheerden
Copy link
Collaborator Author

log-2023-11-23_11-47-03.txt

@SWvheerden
Copy link
Collaborator Author

might be related to #5974

@SWvheerden SWvheerden added release-blocker Something that needs to be fixed before a release can be made and removed P-high-risk Process - High risk labels Nov 27, 2023
@SWvheerden
Copy link
Collaborator Author

SWvheerden commented Nov 27, 2023 •
edited
Loading

log-2023-11-26_20-44-57-crash_after_payments.txt

@SWvheerden SWvheerden mentioned this issue Nov 27, 2023
Closed
@hansieodendaal hansieodendaal self-assigned this Nov 28, 2023
@hansieodendaal hansieodendaal mentioned this issue Dec 4, 2023
Merged
4 tasks
@hansieodendaal
Copy link
Contributor

See #6007

SWvheerden pushed a commit that referenced this issue Dec 7, 2023
@hansieodendaal
feat: add wallet ffi shutdown tests (#6007)
3129ce8 
Description
---
- Added wallet FFI shutdown tests to prove that all services have been
terminated when the FFI `wallet_destroy` command has been run.
- Added additional safeguards in the FFI `wallet_destroy` method to
ensure the wallet has shut down.

Motivation and Context
---
See #5984

How Has This Been Tested?
---
Added `pub fn test_wallet_shutdown()` in the wallet FFI library.

What process can a PR reviewer use to test or verify this change?
---
- Review test code
- Run the test

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


Breaking Changes
---

- [x] None
- [ ] Requires data directory on base node to be deleted
- [ ] Requires hard fork
- [ ] Other - Please specify

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->
@SWvheerden SWvheerden reopened this Dec 12, 2023
@SWvheerden
Copy link
Collaborator Author

log-2023-12-12_13-46-06.txt

@SWvheerden
Copy link
Collaborator Author

not fixed yet

@hansieodendaal
Copy link
Contributor

From the mobile wallet log:

These are the usual log entries after the shutdown signal disconnectWallet Start: UIBackgroundTaskIdentifier log entry:

2023-12-12 14:04:26.506942000 [wallet_ffi] DEBUG [Aurora] | Debug             | INFO    | disconnectWallet Start: UIBackgroundTaskIdentifier(rawValue: 95)
2023-12-12 14:04:26.508763000 [contacts::contacts_service] INFO  Contacts service shutting down because it received the shutdown signal
2023-12-12 14:04:26.509089000 [contacts::contacts_service] INFO  Contacts Service ended
2023-12-12 14:04:26.509118000 [contacts::contacts_service::initializer] INFO  Contacts service shutdown
2023-12-12 14:04:26.509151000 [wallet::base_node_service::service] INFO  Wallet Base Node Service shutting down because the shutdown signal was received
2023-12-12 14:04:26.509179000 [wallet::base_node_service] INFO  Wallet Base Node Service shutdown with result Ok(())
2023-12-12 14:04:26.509201000 [wallet::utxo_scanning] INFO  UTXO scanning service shutting down because it received the shutdown signal
2023-12-12 14:04:26.509237000 [wallet::utxo_scanner_service::initializer] INFO  Utxo scanner service shutdown
2023-12-12 14:04:26.511493000 [wallet::transaction_service::protocols::send_protocol] INFO  Transaction Send Protocol (id: 4126586376629469630) shutting down because it received the shutdown signal
2023-12-12 14:04:26.511564000 [wallet::transaction_service::service] INFO  Transaction service shutting down because it received the shutdown signal
2023-12-12 14:04:26.511607000 [wallet::transaction_service::service] INFO  Transaction service shut down
2023-12-12 14:04:26.512120000 [wallet::output_manager_service] INFO  Output manager service shutting down because it received the shutdown signal
2023-12-12 14:04:26.512893000 [wallet::output_manager_service] INFO  Output Manager Service ended
2023-12-12 14:04:26.512922000 [wallet::output_manager_service::initializer] INFO  Output manager service shutdown
2023-12-12 14:04:26.521537000 [wallet::transaction_service] INFO  Transaction Service shutdown

The last entry of disconnectWallet Start: UIBackgroundTaskIdentifier was not followed with the usual shutdown pattern with shutdown log entries from the different services, suggesting that the shutdown command has not been issued in the FFI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hansieodendaal hansieodendaal

Labels
release-blocker Something that needs to be fixed before a release can be made
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@SWvheerden @hansieodendaal