Fixing Win64/CI unit test segfault

why:
  Previously, the function 'snapshot_desc.loadSnapshot()' contained the
  equivalent of 'eth.decode(@[],SnapshotData)' for some type 'SnapshotData'
  which should result in an exception of type 'RlpTypeMismatch'.

  Before mid October, this worked for all systems on the Github CI. Since
  then, a segfault message in the Github CI can be reproduced on all 64bit
  Windows wuns when running 'build/all_tests <id-of-test_txpool>' after the
  failed 'make test' directive (the latter one needs to be extended by
  '|| true'.)  This error cannot be reproduced on my local Win7/64 system
  with the same MSYS2 and gcc 11.2.0 compiler.

  The fix is, rather than catching an exception, to explicitly check the
  first argument of 'eth.decode(@[],SnapshotData)' and act if it is empty.

also:
  removed some obsolete {.inline.} annotations.
This commit is contained in:
Jordan Hrycaj 2022-01-12 15:55:36 +00:00 committed by zah
parent 9545767c53
commit 1eb79c34c6
3 changed files with 63 additions and 46 deletions

View File

@ -63,7 +63,7 @@ proc persistBlocksImpl(c: Chain; headers: openarray[BlockHeader];
let
validationResult = vmState.processBlock(c.clique, header, body)
when not defined(release):
if validationResult == ValidationResult.Error and
body.transactions.calcTxRoot == header.txRoot:

View File

@ -66,13 +66,20 @@ logScope:
# Private debugging functions, pretty printing
# ------------------------------------------------------------------------------
proc say(d: var LocalSnaps; v: varargs[string,`$`]) {.inline.} =
template say(d: var LocalSnaps; v: varargs[untyped]): untyped =
discard
# uncomment body to enable
#d.c.cfg.say v
# uncomment body to enable, note that say() prints on <stderr>
# d.c.cfg.say v
proc pp(a: Hash256): string =
if a == BLANK_ROOT_HASH:
"*blank-root*"
elif a == EMPTY_SHA3:
"*empty-sha3*"
else:
a.data.mapIt(it.toHex(2)).join[56 .. 63].toLowerAscii
proc pp(q: openArray[BlockHeader]; n: int): string {.inline.} =
proc pp(q: openArray[BlockHeader]; n: int): string =
result = "["
if 5 < n:
result &= toSeq(q[0 .. 2]).mapIt("#" & $it.blockNumber).join(", ")
@ -81,38 +88,40 @@ proc pp(q: openArray[BlockHeader]; n: int): string {.inline.} =
result &= toSeq(q[0 ..< n]).mapIt("#" & $it.blockNumber).join(", ")
result &= "]"
proc pp(b: BlockNumber, q: openArray[BlockHeader]; n: int): string {.inline.} =
proc pp(b: BlockNumber, q: openArray[BlockHeader]; n: int): string =
"#" & $b & " + " & q.pp(n)
proc pp(q: openArray[BlockHeader]): string {.inline.} =
proc pp(q: openArray[BlockHeader]): string =
q.pp(q.len)
proc pp(b: BlockNumber, q: openArray[BlockHeader]): string {.inline.} =
proc pp(b: BlockNumber, q: openArray[BlockHeader]): string =
b.pp(q, q.len)
proc pp(h: BlockHeader, q: openArray[BlockHeader]; n: int): string {.inline.} =
proc pp(h: BlockHeader, q: openArray[BlockHeader]; n: int): string =
"headers=(" & h.blockNumber.pp(q,n) & ")"
proc pp(h: BlockHeader, q: openArray[BlockHeader]): string {.inline.} =
proc pp(h: BlockHeader, q: openArray[BlockHeader]): string =
h.pp(q,q.len)
proc pp(t: var LocalPath; w: var LocalSubChain): string {.inline.} =
proc pp(t: var LocalPath; w: var LocalSubChain): string =
var (a, b) = (w.first, w.top)
if a == 0 and b == 0: b = t.chain.len
"trail=(#" & $t.snaps.blockNumber & " + " & t.chain[a ..< b].pp & ")"
proc pp(t: var LocalPath): string {.inline.} =
proc pp(t: var LocalPath): string =
var w = LocalSubChain()
t.pp(w)
proc pp(err: CLiqueError): string =
"(" & $err[0] & "," & err[1] & ")"
# ------------------------------------------------------------------------------
# Private helpers
# ------------------------------------------------------------------------------
proc maxCheckPointLe(d: var LocalSnaps;
number: BlockNumber): BlockNumber {.inline.} =
proc maxCheckPointLe(d: var LocalSnaps; number: BlockNumber): BlockNumber =
let epc = number mod d.c.cfg.ckpInterval
if epc < number:
number - epc
@ -120,16 +129,13 @@ proc maxCheckPointLe(d: var LocalSnaps;
# epc == number => number < ckpInterval
0.u256
proc isCheckPoint(d: var LocalSnaps;
number: BlockNumber): bool {.inline.} =
proc isCheckPoint(d: var LocalSnaps; number: BlockNumber): bool =
(number mod d.c.cfg.ckpInterval) == 0
proc isEpoch(d: var LocalSnaps;
number: BlockNumber): bool {.inline.} =
proc isEpoch(d: var LocalSnaps; number: BlockNumber): bool =
(number mod d.c.cfg.epoch) == 0
proc isSnapshotPosition(d: var LocalSnaps;
number: BlockNumber): bool {.inline.} =
proc isSnapshotPosition(d: var LocalSnaps; number: BlockNumber): bool =
# clique/clique.go(394): if number == 0 || (number%c.config.Epoch [..]
if d.isEpoch(number):
if number.isZero:
@ -147,7 +153,7 @@ proc isSnapshotPosition(d: var LocalSnaps;
# ------------------------------------------------------------------------------
proc findSnapshot(d: var LocalSnaps): bool
{.inline, gcsafe, raises: [Defect,CatchableError].} =
{.gcsafe, raises: [Defect,CatchableError].} =
## Search for a snapshot starting at current header starting at the pivot
## value `d.start`. The snapshot returned in `trail` is a clone of the
## cached snapshot and can be modified later.
@ -166,7 +172,7 @@ proc findSnapshot(d: var LocalSnaps): bool
let number = header.blockNumber
# Check whether the snapshot was recently visited and cahed
# Check whether the snapshot was recently visited and cached
if d.c.recents.hasLruSnaps(hash):
let rc = d.c.recents.getLruSnaps(hash)
if rc.isOK:
@ -184,7 +190,7 @@ proc findSnapshot(d: var LocalSnaps): bool
let rc = d.c.cfg.loadSnapshot(hash)
if rc.isOk:
d.trail.snaps = rc.value.cloneSnapshot
d.say "findSnapshot disked ", d.trail.pp
d.say "findSnapshot on disk ", d.trail.pp
trace "Loaded voting snapshot from disk",
blockNumber = number,
blockHash = hash
@ -196,7 +202,8 @@ proc findSnapshot(d: var LocalSnaps): bool
if d.isSnapshotPosition(number):
# clique/clique.go(395): checkpoint := chain.GetHeaderByNumber [..]
d.trail.snaps = d.c.cfg.newSnapshot(header)
if d.trail.snaps.storeSnapshot.isOK:
let rc = d.trail.snaps.storeSnapshot
if rc.isOK:
d.say "findSnapshot <epoch> ", d.trail.pp
info "Stored voting snapshot to disk",
blockNumber = number,
@ -230,7 +237,7 @@ proc findSnapshot(d: var LocalSnaps): bool
proc applyTrail(d: var LocalSnaps): CliqueOkResult
{.inline, gcsafe, raises: [Defect,CatchableError].} =
{.gcsafe, raises: [Defect,CatchableError].} =
## Apply any `trail` headers on top of the snapshot `snap`
if d.subChn.first < d.subChn.top:
block:
@ -239,7 +246,8 @@ proc applyTrail(d: var LocalSnaps): CliqueOkResult
let rc = d.trail.snaps.snapshotApplySeq(
d.trail.chain, d.subChn.top-1, d.subChn.first)
if rc.isErr:
d.say "applyTrail snaps=#",d.trail.snaps.blockNumber, " err=",$rc.error
d.say "applyTrail snaps=#", d.trail.snaps.blockNumber,
" err=", rc.error.pp
return err(rc.error)
d.say "applyTrail snaps=#", d.trail.snaps.blockNumber
@ -250,7 +258,7 @@ proc applyTrail(d: var LocalSnaps): CliqueOkResult
if rc.isErr:
return err(rc.error)
d.say "updateSnapshot <disk> chechkpoint #", d.trail.snaps.blockNumber
d.say "applyTrail <disk> chechkpoint #", d.trail.snaps.blockNumber
trace "Stored voting snapshot to disk",
blockNumber = d.trail.snaps.blockNumber,
blockHash = d.trail.snaps.blockHash
@ -258,7 +266,7 @@ proc applyTrail(d: var LocalSnaps): CliqueOkResult
proc updateSnapshot(d: var LocalSnaps): SnapshotResult
{.gcsafe, raises: [Defect,CatchableError].} =
{.gcsafe, raises: [Defect,CatchableError].} =
## Find snapshot for header `d.start.header` and assign it to the LRU cache.
## This function was expects thet the LRU cache already has a slot allocated
## for the snapshot having run `getLruSnaps()`.
@ -406,7 +414,7 @@ proc cliqueSnapshot*(c: Clique;hash: Hash256;
c.cliqueSnapshotSeq(hash,list)
proc cliqueSnapshot*(c: Clique; header: Blockheader): SnapshotResult
{.inline,gcsafe,raises: [Defect,CatchableError].} =
{.gcsafe,raises: [Defect,CatchableError].} =
## Short for `cliqueSnapshot(c,header,@[])`
var blind: seq[Blockheader]
c.cliqueSnapshotSeq(header, blind)

View File

@ -80,13 +80,13 @@ proc signersList(s: Snapshot): string =
# Private functions needed to support RLP conversion
# ------------------------------------------------------------------------------
proc append[K,V](rw: var RlpWriter; tab: Table[K,V]) {.inline.} =
proc append[K,V](rw: var RlpWriter; tab: Table[K,V]) =
rw.startList(tab.len)
for key,value in tab.pairs:
rw.append((key,value))
proc read[K,V](rlp: var Rlp;
Q: type Table[K,V]): Q {.inline, raises: [Defect,CatchableError].} =
Q: type Table[K,V]): Q {.raises: [Defect,CatchableError].} =
for w in rlp.items:
let (key,value) = w.read((K,V))
result[key] = value
@ -165,23 +165,23 @@ proc newSnapshot*(cfg: CliqueCfg; header: BlockHeader): Snapshot =
# Public getters
# ------------------------------------------------------------------------------
proc cfg*(s: Snapshot): CliqueCfg {.inline.} =
proc cfg*(s: Snapshot): CliqueCfg =
## Getter
s.cfg
proc blockNumber*(s: Snapshot): BlockNumber {.inline.} =
proc blockNumber*(s: Snapshot): BlockNumber =
## Getter
s.data.blockNumber
proc blockHash*(s: Snapshot): Hash256 {.inline.} =
proc blockHash*(s: Snapshot): Hash256 =
## Getter
s.data.blockHash
proc recents*(s: Snapshot): var AddressHistory {.inline.} =
proc recents*(s: Snapshot): var AddressHistory =
## Retrieves the list of recently added addresses
s.data.recents
proc ballot*(s: Snapshot): var Ballot {.inline.} =
proc ballot*(s: Snapshot): var Ballot =
## Retrieves the ballot box descriptor with the votes
s.data.ballot
@ -189,11 +189,11 @@ proc ballot*(s: Snapshot): var Ballot {.inline.} =
# Public setters
# ------------------------------------------------------------------------------
proc `blockNumber=`*(s: Snapshot; number: BlockNumber) {.inline.} =
proc `blockNumber=`*(s: Snapshot; number: BlockNumber) =
## Getter
s.data.blockNumber = number
proc `blockHash=`*(s: Snapshot; hash: Hash256) {.inline.} =
proc `blockHash=`*(s: Snapshot; hash: Hash256) =
## Getter
s.data.blockHash = hash
@ -207,13 +207,22 @@ proc loadSnapshot*(cfg: CliqueCfg; hash: Hash256):
## Load an existing snapshot from the database.
var s = Snapshot(cfg: cfg)
try:
s.data = s.cfg.db.db
.get(hash.cliqueSnapshotKey.toOpenArray)
.decode(SnapshotData)
let rlpData = s.cfg.db.db.get(hash.cliqueSnapshotKey.toOpenArray)
# The following check is only needed for Github/CI for 64bit Windows (not
# reproducible on my local Win7 -- jordan). What normally happens when
# `rlpData == @[]` holds is that the library function `eth.readImpl()`
# throws an `RlpTypeMismatch` error as the inner function `isList()` fails.
# On Github CI for 64bit Windows, the unit test crashes with a segmentation
# fault.
if rlpData.len == 0:
return err((errSnapshotLoad,""))
s.data = rlpData.decode(SnapshotData)
s.data.ballot.debug = s.cfg.debug
except CatchableError as e:
return err((errSnapshotLoad,e.msg))
result = ok(s)
return err((errSnapshotLoad, $e.name & ": " & e.msg))
ok(s)
# clique/snapshot.go(104): func (s *Snapshot) store(db [..]
proc storeSnapshot*(s: Snapshot): CliqueOkResult {.gcsafe,raises: [Defect].} =
@ -222,14 +231,14 @@ proc storeSnapshot*(s: Snapshot): CliqueOkResult {.gcsafe,raises: [Defect].} =
s.cfg.db.db
.put(s.data.blockHash.cliqueSnapshotKey.toOpenArray, rlp.encode(s.data))
except CatchableError as e:
return err((errSnapshotStore,e.msg))
result = ok()
return err((errSnapshotStore, $e.name & ": " & e.msg))
ok()
# ------------------------------------------------------------------------------
# Public deep copy
# ------------------------------------------------------------------------------
proc cloneSnapshot*(s: Snapshot): Snapshot {.inline.} =
proc cloneSnapshot*(s: Snapshot): Snapshot =
## Clone the snapshot
Snapshot(
cfg: s.cfg, # copy ref