mastercoin-MSC / mastercore

mastercore info
mastercoin.org
MIT License
24 stars 11 forks source link

Fire -alertnotify, shutdown with message #244

Closed dexX7 closed 9 years ago

dexX7 commented 9 years ago

This is no functional change and has basically no effect at this point, because the whole alert notification system isn't active.

However, if it were, then:

zathras-crypto commented 9 years ago

Just so it's clear how that particular update case is used (sorry if repeating myself, just for clarity): Use isTransactionTypeAllowed with the blockheight and tx type/version in the global_alert_message string. If true, do nothing - client supports the new tx If false and tip is < blockheight, display the text part of global_alert_message as an alert string If false and tip is > blockheight, force a shutdown because txs are now live that our client doesn't support

zathras-crypto commented 9 years ago

TL:DR; we want to fire alertnotify when we receive an alert, not just when we force a shutdown because of lack of tx support - alerts can expire by time, block, version and it's only one specific case that we force a shutdown, we should trigger alertnotify on alert receipt :)

dexX7 commented 9 years ago

Yeah, I tend to simply close this for a more general solution. Isn't very useful as it is right now, but I wanted to get the ball rolling, so to speak. ;)

Edit: pushing one on shutdown is the last resort. For a server it might look like a client crash, if it simply shuts down. A potential challenge re: "fire on receive" is to deal with historical alerts while parsing from zero to tip, basically old alerts should be skipped, if non relevant etc.

dexX7 commented 9 years ago

Here is basically how it looks like during shutdown:

141214_console

141214

dexX7 commented 9 years ago

I'll close this, since the direction is set to a route which doesn't require a "last resort" solution. :)

dexX7 commented 9 years ago

I noticed on this machine that I could trigger a segfault a few times, which were related to a LOCK failure during startup while using CAlert::Notify with fThread = true, similar to how it's done in this PR, but I'm unable to reproduce it now (some hours later), probably due to the asynchrone nature of a multi thread environment. The key ingredient was to start in a state that is doomed to fail:

The strategy in this context is to shutdown immediently - and in this particular case to spawn a new thread and execute a command specified by -alertnotify=%s. Due to the order of how Master Core is initialized this can occure during or before initialization of other modules.

The lock failure could be traced back to the node startup, and to be more specific, to:

StartNode(threadGroup)
-> Discover(threadGroup)
-> ThreadGetMyExternalIP()
-> AddLocal(addrLocalHost, LOCAL_HTTP)
-> AddLocal(CService(addr, GetListenPort()), nScore)
-> IsLimited(addr)
-> IsLimited(addr.GetNetwork())
-> LOCK(cs_mapLocalHost)

Now all those specifics are not relevant, but I was actually surprised about the overlap and much action even before the client was fully initialized.

So what does this mean? I'm actually not sure. I guess it comes down to: care should be taken related to (un-) safe calls and futhermore it is notable that one or many successful runs don't imply fail safe or robust code. ;)

dexX7 commented 9 years ago

So there is doc/coding.md#lockingmutex-usage-notes and a compile option -DDEBUG_LOCKORDER to get lock order inconsistencies reported in the debug.log file.

Unfortunally I'm unable to reconstruct a segfault and failure at the moment, but I saved the crash report (without core dump):

https://gist.githubusercontent.com/dexX7/19ef9933634e02bc8639/raw/

dexX7 commented 9 years ago

Superseded by #246.