Adding the following to bitcoin.conf
[xxx]
disablewallet=1
And running bitcoin-qt gives:
libc++abi.dylib: terminating with uncaught exception of type boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::signals2::no_slots_error> >: boost::signals2::no_slots_error
Fixes regression in #14708.
690 | @@ -691,6 +691,10 @@ int main(int argc, char *argv[]) 691 | #endif 692 | // Install qDebug() message handler to route to debug.log 693 | qInstallMessageHandler(DebugMessageHandler); 694 | + 695 | + // Redundantly log and print message in non-gui fashion 696 | + auto handler_message_box = node->handleMessageBox(noui_ThreadSafeMessageBox);
why assign this to a variable when it's not going to be used?
Internally handler_message_box has a boost::signals2::scoped_connection and the destructor disconnects.
might want to mention this in the comment, that is quite the gotcha
Alternatively could call noui_connect in src/qt/bitcoin.cpp?
I think the safest solution to this is to not call the message box at all for unrecognized sections. Simply call LogPrintf directly. That's what will happen after all!
Updated, @MarcoFalke you might want to check this out since you reviewed #14708.
Concept ACK
So to be clear, the problem is that at this stage of initialization, we're not yet able to show message boxes in Qt [1]. @promag's initial fix changed the slot handler to first register a message box handler that only logs, but, it seems safer to simply change these InitWarning calls to LogPrintf as that's what is going to happen both for the UI and bitcoind.
~May as well match the form of the other warnings in this function, no? They print a leading LogPrintf("%s: ...", __func__, ...).~
Now using fprintf(stderr, ...) Instead of LogPrintf so that test/functional/feature_config_args.py passes.
810 | }
811 |
812 | // Warn if unrecognized section name are present in the config file.
813 | for (const auto& section : gArgs.GetUnrecognizedSections()) {
814 | - InitWarning(strprintf(_("Section [%s] is not recognized."), section));
815 | + fprintf(stderr, "Warning: Section [%s] is not recognized.\n", section.c_str());
InitWarning also prints to the debug log, fprintf does not.
Slightly tend to NACK. This makes the code fragile in light of refactoring such as move-only commits, since it is not clear when InitWarning is safe to call. Wouldn't it be easier to always register the no_ui listener to the signal as soon as possible in the initialization sequence (regardless of bitcoind vs bitcoin-qt) and then optionally, if the gui is run, also register the gui message box listener?
Side-note: Previously we wouldn't even log nor print warnings when running the gui, but that is now fixed (see #14162)
Previous fix in 91ce5a2, which @MarcoFalke disapproved.
Current fix does what was suggested in #14783 (comment).
Not sure about this because I think it should go thru interfaces::Node?
Should remove the now-redundant (?) static void InitMessage in src/qt/bitcoin.cpp?
I'm still not sure about this, but ok…
For example:
ThreadSafeMessageBox, will registering multiple handlers cause an issue here (similar for other handlers that noui_register registers)
@laanwj you mean noui_connect instead of noui_register. Regarding:
- Does noui_register register anything else that might interfere with the GUI?
noui_connect only connects 3 slots that only call LogPrintf and fprintf, so I think we are safe.
- What about the return argument of
ThreadSafeMessageBox, will registering multiple handlers cause an issue here (similar for other handlers thatnoui_registerregisters)
In boost::signals, slot results are combined to give the signal result. The default combiner is to give the last slot result. So the first connected slots are those in noui_connect and later are the UI slots, which means the signal result comes from the UI slots. So it's safe to say there is no behavior change.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
557 | @@ -563,6 +558,11 @@ int main(int argc, char *argv[]) 558 | 559 | std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(); 560 | 561 | + // Subscribe to global signals from core 562 | + std::unique_ptr<interfaces::Handler> handler = node->handleMessageBox(noui_ThreadSafeMessageBox); 563 | + std::unique_ptr<interfaces::Handler> handler = node->handleQuestion(noui_ThreadSafeQuestion); 564 | + std::unique_ptr<interfaces::Handler> handler = node->handleInitMessage(noui_InitMessage);
This does not compile.
Fixed.
Concept ACK
Should remove the now-redundant (?) static void InitMessage in src/qt/bitcoin.cpp? @MarcoFalke done.
557 | @@ -563,6 +558,11 @@ int main(int argc, char *argv[]) 558 | 559 | std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(); 560 | 561 | + // Subscribe to global signals from core 562 | + std::unique_ptr<interfaces::Handler> handler_message_box = node->handleMessageBox(noui_ThreadSafeMessageBox); 563 | + std::unique_ptr<interfaces::Handler> handler_question = node->handleQuestion(noui_ThreadSafeQuestion); 564 | + std::unique_ptr<interfaces::Handler> handler_init_message = node->handleInitMessage(noui_InitMessage); 565 | + 566 | // Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory
What makes it safe to subscribe to them here already, when some of then log to the debug log, which is in the datadir?
If that was the case then boost::signals2::no_slots_error would have been raised before?
utACK 6bbdb2077eb4dae9f7a1938bdbcffab884719cbe. Going to test the next time I compile the gui
I can reproduce the error on macOS 10.14.1 and confirm 6bbdb20 fixes it. It shows a Section [xxx] is not recognized. after the command, but someone launching QT via a desktop icon wouldn't see that.
Would be nice to have to test, though that might require #11625