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

Use shared_ptr to WS connection in API connection #119

Merged
merged 17 commits into from May 10, 2019

Conversation

abitmore
Copy link
Member

@abitmore abitmore commented Apr 3, 2019

For bitshares/bitshares-core#1690

My opinion: don't merge this unless we've found a way to fix the side effects: bitshares/bitshares-core#1695 (comment)
Update: already fixed the side effects along with some other CLI improvements, E.G.

  • pressing TAB key will complete a command as long as possible
  • Increased CLI command history buffer size to 256 (from 15)
  • CLI no longer stores continuous duplicate commands to history, so no longer need to press UP key several times to find a different command
  • pressing Ctrl+C will quit CLI cleanly at most time
  • sending SIGINT or SIGTERM to CLI will terminate it cleanly at most time
  • when CLI got disconnected by the API node, it will quit cleanly at most time

By the way,

  • do you know we can press Ctrl+R to search in command history? Although it's a bit buggy.
  • Do we want a feature that saves command history to a file e.g. on quit or on the fly and load it on next startup?

@abitmore
Copy link
Member Author

abitmore commented Apr 3, 2019

When using a reference (the old code), it's possible that the referenced websocket_connection object has been destructed elsewhere when the websocket_api_connection object trying to dereference it, e.g. when calling websocket_api_connection::send_call(), which would cause a memory violation error.

After changed to a shared_ptr (this PR), it's guaranteed that the websocket_connection object will be always alive.

@abitmore abitmore marked this pull request as ready for review April 3, 2019 18:03
For example, if there are 3 commands: "gethelp", "get_account" and "get_account_name",
input "ge" then press TAB, the command will be completed to "get";
press TAB again, a list with all 3 commands will show;
input "get_ac" then press TAB, the command will be completed to "get_account";
press TAB again, a list with "get_account" and "get_account_name" will show.
@abitmore abitmore mentioned this pull request May 7, 2019
@abitmore
Copy link
Member Author

abitmore commented May 7, 2019

Fixed signal handling in CLI. Need troglobit/editline#29.

@abitmore
Copy link
Member Author

abitmore commented May 8, 2019

I think this PR is ready for merge, although there are still a few issues existing in editline (e.g. troglobit/editline#26, troglobit/editline#30, troglobit/editline#31), we can bump editline later when they're fixed, but there is nothing we can do right now.

By the way, check_secret is removed due to dumping of editline. If there is no progress on #97, I'll work on it.

OP updated.

src/rpc/cli.cpp Outdated Show resolved Hide resolved
@abitmore abitmore merged commit 67e5a06 into master May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants