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

cli_wallet throws erorr and crashes #1127

Closed
bobinson opened this issue Jul 7, 2018 · 21 comments · Fixed by bitshares/bitshares-fc#60
Closed

cli_wallet throws erorr and crashes #1127

bobinson opened this issue Jul 7, 2018 · 21 comments · Fixed by bitshares/bitshares-fc#60
Assignees
Labels
1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2d Developing Status indicating currently designing and developing a solution 3d Bug Classification indicating the existing implementation does not match the intention of the design 4b Normal Priority Priority indicating the moderate impact to system/user -OR- existing workaround is costly to perform 6 CLI Impact flag identifying the command line interface (CLI) wallet application

Comments

@bobinson
Copy link

bobinson commented Jul 7, 2018

  1. cli_wallet version
Version: 2.0.180612
SHA: 27d2e3e3df71d937e7ad5f6f60254de8033d72cd
Timestamp: 25 days ago
SSL: OpenSSL 1.0.2n  7 Dec 2017
Boost: 1.60
Websocket++: 0.7.0

OS: macOS 10.13.5
2. run cli_wallet and connect to localhost

  1. press TAB and the cli_wallet crashes with the following error
Please use the set_password method to initialize a new wallet before continuing
new >>> scli_wallet(7722,0x70000b0fa000) malloc: *** error for object 0x7fbf9d609de0: incorrect checksum for freed object - object was probably modified after being freed.
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6
@oxarbitrage
Copy link
Member

i just tried it on linux and when tab is pressed i get a list of all commands available. should be something regarding the editline in macos, probably @jmjatlanta can reproduce and confirm.

@bobinson
Copy link
Author

bobinson commented Jul 7, 2018

Further I observed that if a TAB is pressed after say, get it prints matching patterns. If no alternate options available say, type s or nothing at all it fails.

@oxarbitrage
Copy link
Member

thanks for the report, we need our mac guy to check it out, i don't have a mac to test this. he is on vacation but will be back in around 1 week if i remember correctly. issue will remain open until we can confirm and submit a solution.
thanks again.

@abitmore abitmore added 3d Bug Classification indicating the existing implementation does not match the intention of the design 4b Normal Priority Priority indicating the moderate impact to system/user -OR- existing workaround is costly to perform 6 CLI Impact flag identifying the command line interface (CLI) wallet application labels Jul 7, 2018
@bobinson
Copy link
Author

bobinson commented Jul 7, 2018

Ok, @oxarbitrage and I compiled a version which works fine.

@oxarbitrage
Copy link
Member

with macOS or with another OS?

@bobinson
Copy link
Author

bobinson commented Jul 9, 2018

same OS and exactly same environment

@oxarbitrage
Copy link
Member

did you made changes to make it work ? want to submit a pull request with the fix ? if so please just make sure you do it against the develop branch.
thanks again.

@ryanRfox ryanRfox added 1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2a Discussion Needed Prompt for team to discuss at next stand up. 0 Awaiting End User Feedback Notification to the Issue creator that the Core Team require additional information to proceed labels Jul 10, 2018
@bobinson
Copy link
Author

@oxarbitrage - I didn't make any code changes. I had downloaded the pre-compiled version first which didn't work. I went ahead and compiled from source (same version) and it worked

@ryanRfox ryanRfox removed the 0 Awaiting End User Feedback Notification to the Issue creator that the Core Team require additional information to proceed label Jul 10, 2018
@ryanRfox
Copy link
Contributor

Thanks for confirming @bobinson how you were able to work around this bug. I too was able to reproduce the error from the binaries.

However, when I compiled from source, the same error resulted as in OP. My environment:

OS: macOS 10.13.5
Version: 2.0.180612-5-g35ec65b1
SHA: 35ec65b130f63c594afe2c9ab7f931b42be08cdc
Timestamp: 22 days ago
SSL: OpenSSL 1.0.2o  27 Mar 2018
Boost: 1.60
Websocket++: 0.7.0

I performed some additional tests (key strokes : result):

  • <TAB> : crash
  • get_w<TAB> : get_witness (success)
  • get_<TAB> : crash
  • get<TAB> : crash

I feel we have a bug in the source code, not limited to the binaries.

@jmjatlanta May I request you to test within your environment if possible?

@jmjatlanta
Copy link
Contributor

Yes, I am getting the same thing. I am investigating.

@bobinson
Copy link
Author

@ryanRfox that's weird! Let me see how I compiled. I have too many versions of boost on my system. Will share the environment once I figure this out.

@jmjatlanta
Copy link
Contributor

Valgrind reports a SIGSEV within pthread. Each time I move code around (i.e. add debugging stuff) the program fails at a different place. That's a sign of a segmentation violation as well. I am still trying to chase down where the problem lies.

If you can figure out how you compiled @bobinson that may give us an indication as to what is causing it. Thanks for the help.

@jmjatlanta
Copy link
Contributor

jmjatlanta commented Jul 11, 2018

It seems there is a segmentation violation in editline.c. I believe the realloc should happen before the passed in char is added. I modified it on my machine and it seems to fix the issue.

https://github.com/troglobit/editline/blob/534b3897b82f2ecb3a9e46272b00a50299972001/src/editline.c#L178-L182

I created an issue with the maintainer. troglobit/editline#18

@ryanRfox
Copy link
Contributor

!!Wow!! That is some serious sleuthing. I see your fix is now merged to the editline master. I'm not sure how that change will propagate down to my build environment. I tried brew update && brew outdated but editline did not show up there. Is that library included within something else?

@bobinson
Copy link
Author

bobinson commented Jul 12, 2018

Environment:
OS : Darwin air.local 17.6.0 Darwin Kernel Version 17.6.0: Tue May 8 15:22:16 PDT 2018; root:xnu-4570.61.1~1/RELEASE_X86_64 x86_64 (10.13.5 (17F77)
Version: 2.0.180612-5-g35ec65b1
SHA: 35ec65b
Timestamp: 24 days ago
SSL: OpenSSL 1.0.2o 27 Mar 2018
Boost: 1.67
Websocket++: 0.7.0

Steps of Trying with the above environment is given below

Also tried this without doing brew update and will try to reproduce the same with the original libboost version too and report.

  1. cmake . from the source directory gives the following messages
-- Using custom FindBoost.cmake
-- Boost version: 1.67.0
-- Found the following Boost libraries:
--   thread
--   iostreams
--   date_time
--   system
--   filesystem
--   program_options
--   signals
--   serialization
--   chrono
--   unit_test_framework
--   context
--   locale
-- Using custom FindBoost.cmake
-- Boost version: 1.67.0
-- Found the following Boost libraries:
--   coroutine
-- Configuring BitShares on OS X
-- Configuring project fc located in: /Users/username/hack/bitshares-core/libraries/fc
-- Configuring fc to build on Unix/Apple
-- Using custom FindBoost.cmake
-- Boost version: 1.67.0
-- Found the following Boost libraries:
--   thread
--   date_time
--   system
--   filesystem
--   program_options
--   signals
--   serialization
--   chrono
--   unit_test_framework
--   context
--   locale
--   iostreams
--   coroutine
CMake Error at /usr/local/Cellar/cmake/3.11.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
  system variable OPENSSL_ROOT_DIR (missing: OPENSSL_INCLUDE_DIR)
Call Stack (most recent call first):
  /usr/local/Cellar/cmake/3.11.1/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
  /usr/local/Cellar/cmake/3.11.1/share/cmake/Modules/FindOpenSSL.cmake:390 (find_package_handle_standard_args)
  libraries/fc/CMakeLists.txt:201 (find_package)


-- Configuring incomplete, errors occurred!
See also "/Users/username/hack/bitshares-core/CMakeFiles/CMakeOutput.log".

  1. To fix the error, exported OpenSSL

export OPENSSL_ROOT_DIR=$(brew --prefix)/Cellar/openssl/1.0.2o_1/

  1. code compiled successfully and able to run cli_wallet without error

[07:58 PM] bobinson@air 🖖 [~/hack/bitshares-core/programs/cli_wallet]./cli_wallet --server-rpc-endpoint wss://eu.openledger.info/ws


  1. TESTING

    <TAB> : works as expected
    get_w<TAB> : get_witness (success)
    get_<TAB> : success
    get<TAB> : success

@ryanRfox
Copy link
Contributor

Thank you @bobinson for providing the build details. I notice the Boost version you compiled with is outside of the currently support range of [1.57 - 1.65].

@jmjatlanta I'm somewhat surprised the FindBoost routine did not return an error. I am fairly confident cmake fails if boost is >= 1.56 but that's a separate issue perhaps. Do let me know how to update/check my build environment to ensure I've picked up the editline update you fixed above.

@bobinson
Copy link
Author

@ryanRfox - yes, aware of that. Will try to compile with one of the supporting versions too.

@jmjatlanta
Copy link
Contributor

jmjatlanta commented Jul 16, 2018

@ryanRfox

Is that library included within something else?

It is a submodule of the fc submodule (see libraries/fc/vendor/editline). We need to test the other changes in the latest version of Editline. If it works well, we can update the submodule to use their latest version.

I'm somewhat surprised the FindBoost routine did not return an error.

We check that the appropriate boost modules are available. We do not check to make sure the version of those modules are within a range. Such a change has advantages and disadvantages. We should discuss.

@ryanRfox
Copy link
Contributor

@jmjatlanta thank you for clarifying how editline is included within this project. Yes, let's discuss all things FindBoost in a new Issue. Again, thanks for making these updates.

@abitmore
Copy link
Member

Still need to bump FC.

@abitmore abitmore reopened this Jul 17, 2018
@abitmore abitmore added 2d Developing Status indicating currently designing and developing a solution and removed 2a Discussion Needed Prompt for team to discuss at next stand up. labels Jul 21, 2018
@abitmore
Copy link
Member

Fixed with #1104.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1b User Story The User Story details a requirement. It may reference a parent Epic. It may reference child Task(s) 2d Developing Status indicating currently designing and developing a solution 3d Bug Classification indicating the existing implementation does not match the intention of the design 4b Normal Priority Priority indicating the moderate impact to system/user -OR- existing workaround is costly to perform 6 CLI Impact flag identifying the command line interface (CLI) wallet application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants