< wumpus>
promag: no, that hasn't been updated for weeks, as the 0.16 milestone was effectively 'high priority for review', so we'll want to go over it next meeting and determine what still belongs there and what not
< wumpus>
I agree it's somewhat strange to have a WIP there
< promag>
are we done regarding feature freeze?
< wumpus>
yes, 0.16 is forked off
< wumpus>
so we can merge new features into master, if they're ready
< gmaxwell>
wumpus: assuming no other issues come up do you think we'll release RC2 as final (now that we know that qt assert is not new and really fringe) or do you think we'd do an RC3 for that alone?
< wumpus>
gmaxwell: as it's only a crash on shutdown and happens only in certain circumstances, I'd say we shouldn't hold up the release just for that
< gmaxwell>
thats my view. it's an assert and it looks like it requires lucky timing and a shutdown when started with reindex.
< wumpus>
right, an assertion fail, not even a proper 'crash'
< gmaxwell>
anyone else see a bunch of connections from 23.92.36.0/24 ?
< Sentineo>
let me check
< Sentineo>
gmaxwell: yes, 5
< Sentineo>
but I see it from tor and all bitcoinj:0.14.5
< esotericnonsense>
same
< esotericnonsense>
i also have 5
< gmaxwell>
Sentineo: what do you mean 'from tor'?
< wumpus>
connections from tor hs would come from 127.0.0.1
< esotericnonsense>
i also seem to have a ton of spy nodes or something odd like that lately
< esotericnonsense>
jsonrpc: A String specifying the version of the JSON-RPC protocol. MUST be exactly "2.0".
< wumpus>
esotericnonsense: sure, ok, but do you know what *specifically* it fails on
< wumpus>
oh simply the version number
< esotericnonsense>
there are other bits in the spec that are REQUIRED but at least in the core dev docs they're not REQUIRED (for the requests at least if not the responses)
< esotericnonsense>
but I know that the developer docs are often outdated
< wumpus>
can you try to lolpatch that to 2.0 and see if it works?
< esotericnonsense>
it doesn't even respond with jsonrpc
< esotericnonsense>
i.e. it's not jsonrpc 1.0, it's just not there :P
< esotericnonsense>
i can yeah but not immediately (a lolhack I suppose would be to reply with whatever was in the request :P)
< wumpus>
we pretty much do 2.0, there might be some small things missing
< wumpus>
for example features such as batching are 2.0 afaik
< esotericnonsense>
indeed
< * esotericnonsense>
now has coffee which was the important bit before trying to rpc2.0ise it
< bitcoin-git>
bitcoin/master c887f87 Clem Taylor: Extend #11583 to include the most common message generated by non-contributing peers (port scanners?)...
< bitcoin-git>
bitcoin/master eaeaa2d Wladimir J. van der Laan: Merge #12342: Extend #11583 to include "version handshake timeout" message...
< esotericnonsense>
wumpus: nah that doesn't work. from looking at the code above (github link) it's clear why
< esotericnonsense>
it interprets the spec very literally. we send error: null when it wants the error field to not exist if there's a result
< esotericnonsense>
from memory i think we might also send result: null when the error is non-null
< bitcoin-git>
[bitcoin] laanwj closed pull request #12342: Extend #11583 to include "version handshake timeout" message (master...master) https://github.com/bitcoin/bitcoin/pull/12342
< * esotericnonsense>
tries hacking error to not exist ...
< wumpus>
in the long run we should move toward compliance with the spec, maybe first a command line option (for backwards compatiblity)
< esotericnonsense>
i agree, you'd need to track the client version though
< esotericnonsense>
it would have to be a commandline option because I think it would make sense to enforce the restrictions on the client at the same time
< wumpus>
I mean to moving to strict json rpc 2.0 only, and keep the hack-1.0 compatibility as a command line option for the forseeable future for backwards compatiblity
< esotericnonsense>
ah sure
< esotericnonsense>
that might be something I could look at in the next few weeks :)
< wumpus>
the way it is now is pretty much a historical mistake, but as a lot of software is written against our peculiar json-rpc dialect we can't and shouldn't completely drop it, but having a spec-compliant mode would be great
< wumpus>
awesome
< wumpus>
btcd has this option: --rpcquirks Mirror some JSON-RPC quirks of Bitcoin Core -- NOTE: Discouraged unless interoperability issues need to be worked around -- O dpm
< wumpus>
I don't know what it exactly changes
< esotericnonsense>
the JSONRPCReplyObj function seems to encapsulate all the problems if you could assume the client is jsonrpc 2.0
< wumpus>
ok
< esotericnonsense>
on the response side at least
< esotericnonsense>
i guess it's less important to be strict about what clients send you, but then you don't want to later have a 'really really jsonrpc2.0' flag
< wumpus>
right might be that bitcoin-cli needs changes as well
< esotericnonsense>
:P
< esotericnonsense>
yes\
< wumpus>
haha no
< * esotericnonsense>
wonders if bitcoind-ncurses would explode if the server were compliant
< * esotericnonsense>
smashes himself in the face with a hammer after remembering how he handles rpc in general
< bitcoin-git>
bitcoin/master eeeb416 murrayn: Remove suggestion to make cloned repository world-writable for Windows build.
< bitcoin-git>
bitcoin/master c345148 Wladimir J. van der Laan: Merge #12322: Docs: Remove step making cloned repository world-writable for Windows build....
< bitcoin-git>
[bitcoin] laanwj closed pull request #12322: Docs: Remove step making cloned repository world-writable for Windows build. (master...doc_change) https://github.com/bitcoin/bitcoin/pull/12322
< bitcoin-git>
[bitcoin] laanwj closed pull request #12325: Use dynamic_cast for downcasting instead of static_cast. (master...use_dynamic_cast_to_downcast) https://github.com/bitcoin/bitcoin/pull/12325
< bitcoin-git>
bitcoin/master a1e1305 James O'Beirne: Clarify help messages for path args to mention datadir prefix...
< bitcoin-git>
bitcoin/master 5460460 James O'Beirne: Add AbsPathForConfigVal to consolidate datadir prefixing for path args...
< bitcoin-git>
bitcoin/master f6cd41d Wladimir J. van der Laan: Merge #12305: [docs] [refactor] Add help messages for datadir path mangling...
< bitcoin-git>
[bitcoin] laanwj closed pull request #12305: [docs] [refactor] Add help messages for datadir path mangling (master...jamesob/conf-flag-path-help) https://github.com/bitcoin/bitcoin/pull/12305
< ossifrage>
Has anyone noticed a reduction in the number of connections with v0.16.x? Normally after ~3ish days of uptime I'd be upto 90 or so peers and with the new version I'm having a hard time keeping 60 connections?
< Dudley>
Should DEFAULT_WALLET_RBF be set to TRUE in wallet.h for version 0.16?
< Randolf>
ossifrage: I wonder if some folks have simply stopped running Bitcoin nodes given the recent downturn in price. Or are you seeing the same number of initial connections as you did before?
< ossifrage>
Randolf, previously I had no problem hitting ~100 connections in a few days and often >120
< ossifrage>
Now I'm only hitting 50-60
< Randolf>
I suppose you've been checking your logs?
< adiabat>
ossifrage: I've also seen a big dropoff in nodes and also network traffic, down 50% or so in the last 2 weeks
< ossifrage>
it could just be fall off in price results in a fall off in the number of nodes
< Randolf>
Markets have been down a lot over the past few weeks (more than two weeks), and so I'm guessing that a number of folks might have chosen to stop running their nodes because of that. I've certainly seen some chatter about that with other cryptocurrencies too from folks getting out while the markets
< Randolf>
drop.
< Randolf>
It could be interesting to find out which countries are seeing the most drops in node runners.
< Randolf>
...assuming that's what's happening.
< Randolf>
It would also be nice to know whether some major ISPs have actively started blocking Bitcoin node traffic.
< rabidus>
i haven't seen any drop in my node peer count (.fi)
< Randolf>
rabidus: Are you using v0.16?
< rabidus>
ah, sry, no.
< Randolf>
rabidus: It's still good to know that you haven't seen a drop. :)
< rabidus>
hmmm, well now i'm very curious to test that
< * Randolf>
smiles
< Randolf>
Maybe I'll set up a node too, and let it run for a week.
< BlueMatt>
wumpus: #12337 has two bugs, the first is a regression (so should likely be tagged 0.16, but should be an easy fix), the second is not a regression, though should also be an easy fix so dunno why we shouldnt just fix it
< wumpus>
BlueMatt: we removed the 0.16 tag from 12337, it doesn't warrant doing another r
< wumpus>
rc
< wumpus>
(which I also explained in my post there)
< BlueMatt>
a regression causing a scarry assert on shutdown?
< wumpus>
it's very rare
< wumpus>
and it's only an assert, and only on shutdown
< BlueMatt>
I mean I ran into it like 3 times during testing yesterday
< BlueMatt>
its only rare if you never quit during startup, afaict
< wumpus>
yeah...
< BlueMatt>
(and other stuff probably does merit an rc3, so might as well, imo)
< wumpus>
yes, if an rc3 turns out to be needed we can include the fix
< BlueMatt>
#12273 could be rather annoying for a user who doesnt notice...it probably doesnt itself warrant an rc3 but that + the two bugs in 12337 might
< BlueMatt>
its a race between eg sendrawtransaction and getrawmempool....someone who does a sendrawtransaction followed by an immediate getrawmempool to...test for something or check fee or something may suddenly start failing
< BlueMatt>
its not exactly critical, especially since it doesnt effect wallet
< wumpus>
right, it seems far-fetched
< BlueMatt>
but its kinda gross
< wumpus>
needs to be fixed but does not warrant another rc
< BlueMatt>
agreed (by itself)
< wumpus>
#12362 is scary though, it's exactly what I was afraid of merging #12266 last minute :/
< gribble>
https://github.com/bitcoin/bitcoin/issues/12362 | bitcoind hits corrupted double-linked list error when running multiple wallet_multiwallet.py tests in parallel · Issue #12362 · bitcoin/bitcoin · GitHub
< BlueMatt>
yea, its rather surprising to me...something is really sketchy there :(
< BlueMatt>
I mean this is why I wanted a full cycle of testing on all the parallell/background stuff, turns out even that seems like it wasnt enough :(
< Dudley>
Discard my question. I guess default for walletrbf is different for RPC then the GUI
< wumpus>
Dudley: yes
< provoostenator>
I'm making a fresh Debian 8 VM for Gitian to see which of the issues that I encountered last week are Debian 9 specific (probably still worth fixing at some point).
< wumpus>
provoostenator: yeah it would help a lot if someone worked through that guide and fixed the things that are no longer correct, it's been too long
< wumpus>
most people who have a working gitian environment dread to touch it again
< provoostenator>
I already left about 10 issues on the docs repo, but I suspect some of it was Debian 9 related.
< bitcoin-git>
bitcoin/master bdb3231 251: Implements a virtual destructor on the BaseRequestHandler class....
< bitcoin-git>
bitcoin/master 1462bde Wladimir J. van der Laan: Merge #12050: [trivial] Implements a virtual destructor on the BaseRequestHandler class....
< provoostenator>
I'm also looking at clarifying how to build on the VM, but sign on the host machine.
< provoostenator>
As well as push to github fro the host machine.
< bitcoin-git>
[bitcoin] laanwj closed pull request #12050: [trivial] Implements a virtual destructor on the BaseRequestHandler class. (master...patch/BaseRequestHandler-virtual-dtor) https://github.com/bitcoin/bitcoin/pull/12050
< provoostenator>
I already have a bash script for that, but need to combine that with the existing script (before or after someone converts that to Python).
< arubi>
it's also possible to set up the bridge stuff using the new `ip` commands, but I wasn't sure if the gitian builder scripts look for ifconfig and friends so I'm also pulling the net-tools package
< arubi>
one plus is that it doesn't require installing an old vmbuilder as root
< BlueMatt>
ok, so found the issue in #12362...trying to find minimal patch
< gribble>
https://github.com/bitcoin/bitcoin/issues/12362 | bitcoind hits corrupted double-linked list error when running multiple wallet_multiwallet.py tests in parallel · Issue #12362 · bitcoin/bitcoin · GitHub
< BlueMatt>
wumpus: you still around? you know this code much better than I
< wumpus>
BlueMatt: yes
< wumpus>
(as in still around, not sure about knowing the code better)
< BlueMatt>
issue is simple: HTTPWorkQueueRun starting in a new thread may take longer to get to the queue->Run() call than it takes for us to go through InterruptHTTPServer(); StopHTTPServer()
< BlueMatt>
so the thread will start after the underlying queue has been deleted
< BlueMatt>
resulting in garbage
< wumpus>
looks like we need a lock
< BlueMatt>
prior to the commit to move the threadGroup to interrupt later, the only thing between VerifyWallets() was threadGroup.interrupt_all(), which was enough to keep from seeing this
< BlueMatt>
but once that was removed if VerifyWallets() fails you essentially just immediately call InterruptHTTPServer() and then StopHTTPRPC(), StopREST(), StopRPC() and StopHTTPServer()
< BlueMatt>
so you just get into a simple race there
< wumpus>
yes, so when interrupthttpserver is run, we first need to wait that it is actually running
< BlueMatt>
easy to test for - add a sleep at the top of HTTPWorkQueueRun and run in valgrind
< BlueMatt>
you'll see it immediately
< BlueMatt>
yea, just need to somehow pause until stuff is running
< wumpus>
also, queue->run shouldn't run if it is already terminated
< BlueMatt>
yea, i think that would be an equivalent fix
< wumpus>
but does this fix the race?
< wumpus>
there's still a small window where threads_running is incrased but queue->run is not yet executed
< BlueMatt>
oh yea I mean its not a sufficient fix
< BlueMatt>
its just a hack for testing
< wumpus>
right
< BlueMatt>
wumpus: do you understand the qt init stuff well enough to fix #12337? (it just needs to have its shutdown match bitcoind - Shutdown() should always get called if AppInitMain() gets called, even if it fails)
< BlueMatt>
I mean I can do a dirty hack for an rc3 that just does it directly but I dont understand the lifetimes well enough to not break things
< wumpus>
I lost track of the qt init stuff to be honest, I used to understand it, but a lot changed
< BlueMatt>
:(
< wumpus>
oh that part, yes that should be easy
< wumpus>
it's interesting because it used to be exactly the other way around
< BlueMatt>
ok, well I'd propose we do that and just fix the 4 outstanding issues and do an rc3, then
< BlueMatt>
yea, that stuff got confused and bitcoind and qt diverged introducing the bug in 12337 :(
< wumpus>
I think that switched around a few times
< BlueMatt>
well also AppInit got split.....
< wumpus>
that shouldn't change this
< wumpus>
the earlier appinits shouldn't require a shutdown
< wumpus>
only the last one
< BlueMatt>
yea, well when there was only one appinit... :p
< wumpus>
as that is what is executed in the daemon process
< wumpus>
the other ones are preparation
< BlueMatt>
so one thing we should maybe consider is a test framework that changes shutdownrequested to just start shutdown after being called N times, and then run for all N 0...inf to make sure we handle all these cases well
< BlueMatt>
cause we now have, what, 2 issues that would be caught by a test like that
< wumpus>
yes it definitely needs tests
< wumpus>
the initialization sequence and shutdown are really undertested
< wumpus>
so for the queue->Run() I don't understand why interrupting it before the Run is hit is a problem, interrupt should set the stopped flag on the queue, so it should just fall through and exit the thread immediately
< wumpus>
or does deletion (which should happen in ShutdownHTTPServer, not Interrupt) somehow not wait for the threads to have stopped?
< BlueMatt>
wumpus: I do not believe ShutdownHTTPServer does any meaningful waiting, no
< wumpus>
then that is likely the issue, let me see
< BlueMatt>
yea, I dont think Interrupt is the issue, I think its Shutdown
< wumpus>
it does workqueue->WaitExit()
< wumpus>
in Shutdown
< BlueMatt>
yea, but if the thread hasnt joined (ie is still waiting to call numThreads++) then the wait is null
< ryanofsky>
#11625 adds some testing for qt init code, fwiw
< wumpus>
ah apparently it doesn't actually join the thread objects
< wumpus>
ryanofsky: good!
< wumpus>
the threads are detached and the handle is thrown away
< BlueMatt>
yea
< wumpus>
ok this is easy to solve
< wumpus>
will PR in a minute
< wumpus>
thanks for finding this, I wonder if I introduced this bug or it somehow snuck in when going from boost::thread to std::thread (probably the former)
< BlueMatt>
alright, well I propose we fix all 4 bugs here and then rc3
< BlueMatt>
wait, i may have another....
< BlueMatt>
yup, ok, 5 bugs
< bitcoin-git>
[bitcoin] laanwj opened pull request #12366: http: Join worker threads before deleting work queue (master...2017_02_httpserver_join) https://github.com/bitcoin/bitcoin/pull/12366
< wumpus>
as for qt, it seems it's already supposed to call Shutdown() even if AppInitMain() fails, don't know what goes wrong
< BlueMatt>
wumpus: [13:15:15] <cfields> BlueMatt: i think i might see the issue
< BlueMatt>
[13:16:58] <cfields> BlueMatt: BitcoinApplication connects requestedShutdown to shutdown(), but SplashScreen connects it to close()
< BlueMatt>
[13:17:32] <cfields> so it looks like we should re-route that after AppInitMain has started
< wumpus>
a signal can be connected to two handlers, I don't see the problem
< wumpus>
that just makes sure that the splash screen is hidden when the shutdown window is shown
< BlueMatt>
ahh, well I didnt look into it that deeply, but if Shutdown() is called, nThreadsServicingQueue should be pretty clearly 0
< BlueMatt>
as it join()s
< wumpus>
I'm fairly sure that this has always worked, when closing during the splash screen the shutdown window is shown and it waits for proper shutdown
< wumpus>
no idea wtf messed this up...
< wumpus>
so the assertion is triggered after exiting main()?
< BlueMatt>
^ fixes two fast-shutdown bugs, hopefully with really-simple patches
< MarcoFalke>
rc2 is basically rc1
< MarcoFalke>
As we need to give users time to test, there is no downside in rc3
< dongcarl>
Hi all, I've been trying to get gitian on LXC working to no avail. I'm running into the same issues kallewoof ran into on Dec 1st, 2017. where the error is "sudo: unable to resolve host gitian / cannot set terminal process group (1): Inappropriate ioctl for device / no job control in this shell"
< dongcarl>
I fixed the unable to resolve host error by modifying the /etc/hosts of the base image
< wumpus>
unable to resolve host is harmless at least
< wumpus>
that's just a warning
< bitcoin-git>
[bitcoin] TheBlueMatt opened pull request #12368: Hold mempool.cs for the duration of ATMP. (master...2018-02-getrawmempool-race) https://github.com/bitcoin/bitcoin/pull/12368
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #12273: rpc: Add back missing cs_main lock in getrawmempool (master...Mf1801-rpcMempoolGetLock) https://github.com/bitcoin/bitcoin/pull/12273
< dongcarl>
What happens with the error is that I get dropped into a root shell, and when I ctrl-d, a loop of "sudo: unknown user: ubuntu" and "sudo: unable to initialize policy plugin" pops up
< BlueMatt>
ok, so #12368 + #12367 + #12366 + the qt bug and then rc3? I think all of those except the last are at least super-trivial patches...and at this rate getting an extra week on testing cycles seems to make sense
< wumpus>
dongcarl: is that perhaps with debian 9?
< dongcarl>
wumpus: the base image is trusty
< wumpus>
no, I mean the outer VM
< dongcarl>
I'm on Arch right now
< dongcarl>
am I understanding this wrong? I thought there were two layers, my laptop (Arch) and the lxc container (Trusty)
< wumpus>
heh you might be the first that tries running gitian on arch
< dongcarl>
wumpus: yeah I wanted to make a PR on how to workaround the kinks
< provoostenator>
dongcarl: I've only used the Debian VM. Not sure if it's worth maintaining documentation for 2 distros.
< provoostenator>
Although I suppose it would make the scripts more robust.
< MarcoFalke>
provoostenator: We already have it for fedora as well
< dongcarl>
provoostenator: I think it'll be nice if the scripts are distro-agnostic
< esotericnonsense>
bah run debian or whatever in a vm and run the lxc within that. solved. :P
< dongcarl>
yeah
< wumpus>
well iI assumed you were following the gitian-building md, it starts out with debian VM in which the LXC based gitian is run
< wumpus>
it would be nice, but not realistic I think
< wumpus>
we have lots of trouble keeping it working in just debian
< dongcarl>
:-/
< provoostenator>
I'm getting better at it though, having nuked and rebuilt a Gitian VM half a dozen times now.
< dongcarl>
So I know the zcash people use gitian+vagrant
< dongcarl>
and vagrant is pretty portable across hosts and drivers
< dongcarl>
but that's a whole other thing
< dongcarl>
I think I know where the problem lies...
< MarcoFalke>
Maybe we can get rid of gitian ...
< sipa>
in theory there shouldn't be that much dependece on the outer host
< sipa>
but that's theory...
< wumpus>
hahaha
< wumpus>
in theory
< MarcoFalke>
sipa: The lxc bridge thing usually breaks
< sipa>
the difference between theory and practice is of course that in theory there is nome, but in practice...
< wumpus>
the lxc setup is quite fragile
< MarcoFalke>
yeah
< wumpus>
maybe it's easier with vagrant or another container hoster, I don't know
< wumpus>
if someone has time to play around with that it'd be appreciated
< arubi>
vagrant requires virtualbox right?
< dongcarl>
arubi: no
< dongcarl>
it can take a lot of drivers
< wumpus>
I thought it was just another user of linux containers, just like lxc and docker
< arubi>
oic
< arubi>
I was using qemu for mine
< dongcarl>
it's orchestration layer on top of multiple drivers, lxc, qemu, etc
< arubi>
oh cool
< dongcarl>
quite easy to write extensions
< wumpus>
that sounds like another layer of complexity
< dongcarl>
wumpus: right, that's why I don't know if people would want vagrant
< wumpus>
the problem is that lxc is hard to set up, so switch to something that is another abstraction layer over multiple kinds of virtualization, just like gitian is already trying to be
< dongcarl>
wumpus: but it's quite well-tested and easy to set up
< wumpus>
gitian as-is can also be used with qemu, lxc or even virtualbox (though no one is doing the latter anymore since Gavin left, I think)
< arubi>
maybe aboriginal linux -> mkroot -> build? :P
< dongcarl>
wumpus: I think the end goal is for gitian to target vagrant, and for vagrant to handle interacting with the drivers
< dongcarl>
which would make gitian work much easier
< dongcarl>
anyways, I'll play around with it and see if it actually delivers
< dongcarl>
(or just makes things more complicated)
< wumpus>
right, that could work I guess
< dongcarl>
quick question: is anyone allowed to submit their gitian sigs or just a limited group of people?
< sipa>
dongcarl: if you plan to regularly dongitian builds foe bitcoin core releases, you're very mufh encouraged to upload your key and do so:)
< sipa>
that n between do and gitian is a typo, i swear!
< dongcarl>
sipa: I'm definitely going to name my PR branch dongitian now ;-)
< bitcoin-git>
[bitcoin] MarcoFalke closed pull request #12349: shutdown: fix crash on shutdown with reindex-chainstate (master...fix-qt-shutdown) https://github.com/bitcoin/bitcoin/pull/12349
< wumpus>
everyone is allowed to submit gitian sigs, the more the better
< dongcarl>
<3
< wumpus>
the only requirement is that you get the build working :)
< BlueMatt>
wumpus: did you have any idea why the qt shit isnt calling Shutdown() reliably or should I try to decipher qt
< wumpus>
no, I have no idea right now
< BlueMatt>
k
< wumpus>
from what I see it should simply work
< cfields>
BlueMatt: see my last comment on that ticket, I think I'm in the right ballpark
< BlueMatt>
cfields: wumpus indicated that didnt seem likely
< cfields>
though admittedly, I tried a million theories and probably crossed myself up in the process
< cfields>
oh?
< BlueMatt>
lol well I currently cant build qt due to the moc bug, so...ugh
< jnewbery>
wumpus: change looks good. I can't reproduce 12362 with your PR
< cfields>
eh? still?
< wumpus>
cfields had a different theory there, I haven't checked that one
< wumpus>
jnewbery: great!
< BlueMatt>
cfields: well I never found any resolution to it? moc doesnt build bitcoin-qt right now....no idea why
< wumpus>
cfields: just that the one about re-attaching the signal was not correct, because it's perfectly valid to attach multiple listeners to a signal (or did I misunderstand what you meant?)
< BlueMatt>
cfields: is it just me or is the issue simply that BitcoinCore::initialize() calls initializeResult(false) if AppInitMain() fails, which calls quit() directly instead of calling shutdown() (which would call Interrupt(), Shutdown(), and then shutdownResult() which calls quit())?
< wumpus>
quit() just exits the main loop
< BlueMatt>
yes, but we thus wont call Shutdown(), I think
< wumpus>
after that it starts the shutdown sequence in a new main loop
< BlueMatt>
oh
< wumpus>
after showing the shutdown window
< BlueMatt>
funny because shutdown() will call Interrupt() and then Shutdown() and then quit()
< wumpus>
that should just work, I tested it a zillion times
< wumpus>
yes it calls quit again
< wumpus>
to exit the new mainloop (which handles exiting)
< BlueMatt>
hmm, ok, well I guess I'm just confused
< wumpus>
FWIW it's structured like this because existing the main window will exit the main loop, so we need a new main loop instance to handle the shutdown window while it's shutting down (in the background). There's certainly different ways to do it, but this used to work at least.
< BlueMatt>
well I'll debug moc a bit more and see if I can get that working again
< cfields>
BlueMatt: the problem as i saw it was that quit() is called _both_ by the failed AppInitMain(), as well as when it gets the shutdown signal.
< cfields>
I can only defer to wumpus as to whether that's an issue or not.
< cfields>
but yes, in all of my tests, exit does wait on the shutdown procedure to finish. I can't force any other result :\
< wumpus>
cfields: wait, we don't stop the shutdown poll timer while shutting down?
< cfields>
hmm?
< wumpus>
BitcoinGUI::detectShutdown()
< wumpus>
does that run while it's in the second main loop (while the shutdown window is visible)?
< wumpus>
if so it will prematurely end that loop becuase, yes, it's shut down
< cfields>
wumpus: ugh, no clue. I didn't even look at that.
< wumpus>
cfields: I thought that's what you mean with 'when it gets the shutdown signal'
< wumpus>
the other one (in shutdownResult) is for *after Shutdown() finished* so that's ok
< wumpus>
and the one in initializeResult() is ok too, it means it goes into the shutdown sequence
< wumpus>
but the timer might create such a weird race issue
< wumpus>
ah, pollShutdownTimer is stopped
< wumpus>
in requestShutdown
< wumpus>
no, that should be ok
< cfields>
wumpus: ah, so the timer sees StartShutdown() and fires requestedShutdown(). I see. I thought that was coming from the close button. So shutdown() isn't an overridden qt function?
< wumpus>
cfields: no :)
< cfields>
heh ok
< wumpus>
shutdown() is just the background thread function that actually calls Interrupt() and Shutdown()
< wumpus>
this is bound to the signal requestedShutdown(), coming from the GUI thread
< cfields>
right. I know we call it, but I thought qt emitted it too as some life-cycle callback
< cfields>
yes, I have a much better idea now, thanks
< dongcarl>
If anyone has a working LXC Trusty gitian container, could they show me `ls -la /dev/tty*'?
< arubi>
dongcarl, are you running a nested lxc then? lxc host and gitian builder lxc inside it? if so fwiw it's not so straight forward
< arubi>
if anybody's up for it, please kill the current running #25110 travis job. it's obsolete a waste to keep folks waiting
< BlueMatt>
yay ubuntu dgaf about security, apparently
< BlueMatt>
well, i guess we knew that
< sipa>
dgaf?
< BlueMatt>
dont give a fuck
< dongcarl>
(kids these days)
< dongcarl>
arubi: no, it's just that I've isolated the problem down to the fact that 'sudo lxc-execute -n gitian -f var/lxc.config -- bash' doesn't work
< dongcarl>
it says 'bash: cannot set terminal process group (1): Inappropriate ioctl for device'
< dongcarl>
when i do s/bash/sh/
< dongcarl>
it says 'sh: 0: can't access tty; job control turned off'
< dongcarl>
so something's wrong with tty somewhere...
< arubi>
I see, sorry, I didn't get this error here
< dongcarl>
I think it's to do with me running a newer version of lxc
< dongcarl>
it complains about the config file being an old format too and I had to look at what key names changed
< arubi>
oh man that sounds like a lot of work once non bleeding edge os's are updated to newer lxc..
< dongcarl>
arubi: yup, which is why I'm starting now...
< arubi>
good luck
< dongcarl>
<3
< BlueMatt>
cfields: were you ever able to materially reproduce #12337?