massive URI-handling / IPC server re-work#1023
massive URI-handling / IPC server re-work#1023Diapolo wants to merge 2 commits intobitcoin:masterfrom Diapolo:IPC-server
Conversation
src/qt/bitcoin.cpp
Outdated
There was a problem hiding this comment.
Is there a possibility to display a message box before the GUI is initialized?
There was a problem hiding this comment.
- Before the bitcoin GUI is initialized, in GUI code, in the main thread: yes you can use the QMessageBox::critical static function
- Before Qt and translation services are initialized: no
In this place here you can't.
|
Please don't open pull requests until you think your code is 100% perfect and tested and ready to go. Before then, you can ask people to look at your code or help out using your github repository. |
|
These might be interesting as an alternative to boost::interprocess (and flakey shared memory queues in general): http://doc.qt.nokia.com/4.7-snapshot/qlocalserver.html They provide a named socket (unix socket on unix, pipe on windows) to communicate between processes on the same machine. Alternatively, boost also supports unix sockets and windows named pipes, however, the difference is not abstracted away like in the Qt implementation so will need #ifdefs. |
|
Rebasing required. |
|
Rebased and re-worked a little. I need someone to test this ;). Needs boost 1.49 with a small edit in boost/interprocess/detail/tmp_dir_helpers.hpp see: https://svn.boost.org/trac/boost/ticket/5392#comment:24 There is currently no need for any hard monkey-patches like #986. |
|
NACK-- not worth requiring boost 1.49 for this, and "a small edit in boost" scares the pants off me. |
|
The small edit is uncommenting 3 lines of code that are already IN the tmp_dir_helpers.hpp ... I really would like to know what your problem with a boost update is? If you could explain it a little it makes it easier for me to understand, thanks. |
|
With the amount of trouble it has given us, I think we can safely conclude that boost::interprocess is not ready for production use yet until upstream gets their act together. Also it seems aimed at much more complex use-cases such as sharing memory and objects instead of signaling simple lines of text between processes. When I have some time I'll try coming up with a QLocalServer/QLocalSocket based implementation. |
|
RE: what's the trouble with requiring everybody upgrade to boost 1.49: boost is probably the hardest of our dependencies to get compiled and working, and somebody running an older version of Linux or OSX that spent a day getting boost 1.46 compiled and working properly to compile bitcoind isn't going to be happy if we tell them "you need 1.49 now, because we need that version to fix a bug in URI handling on Windows." They may not care about bitcoin-qt at all and certainly don't care about Windows.... |
|
Gavin: I don't completely follow this reasoning. URL handling (and any usage of boost::interprocess) is limited to bitcoin-qt, if we required boost 1.49 for bitcoin-qt for Windows doesn't mean everyone has to upgrade. The rest of the code can remain backwards compatible. Also, even for bitcoin-qt, the older boost::interprocess works fine in Linux. |
|
It's to easy to say Linux / OSX users may not care about Bitcoin-Qt at all and certainly don't care about Windows. How many Windows users download the client and how many use the GUI version contra how many Linux users are out there? For example I never used bitcoind as Windows users like GUIs and that's a fact :). I could say I don't care about Linux users that want to compile bitcoind, but I don't do this :D. But I'm fine with my work currently not beeing used ... even if no one ever tested it ;). |
|
wtf is with 1e39376? you just committed conflicts! |
|
Dunno how that happened, I normally don't include rebase-conflicts ;), should work again. Sorry @luke-jr! |
src/qt/bitcoin.cpp
Outdated
There was a problem hiding this comment.
This is redundant, boost already deals with this for us, see message_queue.hpp:501 (in 1.49)
"//Check if buffer is smaller than maximum allowed"
|
Updated to reflect the last suggestions from the discussion, all commits will be merged after this get's final (if it get's final ^^), so I used NO speaking commit message! |
|
I will cherry-pick some of the changes and open a new pull. |
1c1488c Revert "Restore lint whitespace check" (Bushstar) Pull request description: Tree-SHA512: c40cea97bece30df9052f9b284ca236185c5b220c012b65ea5211d7808bd52ae98c84850fcd30f11a92c36d8c6204879b851e5ec94b19a0bc590ebc751c85ac7
5 participants
What I try to achieve here:
What I achieved (running and verified on Windows):
What I could not achieve:
Some details:
The message queue is not removed anymore when closing the client, as this causes massive handling problems on Windows with 2 instances and my former approach of stale mq detection. First try is always to open the mq file for a re-use. If this fails a new mq is created.
This needs boost 1.49 with a small edit in boost/interprocess/detail/tmp_dir_helpers.hpp see: https://svn.boost.org/trac/boost/ticket/5392#comment:24
There is currently no need for any hard monkey-patches like #986.