Skip to content

Commit 61f4b9b

Browse files
committed
Manage exceptions in bcc callback functions
Exceptions are not propagated in ctype callback functions used by bcc. This means an AssertionError exception raised by check_equal() to signal a failed assertion is not getting caught and properly logged. Instead, the error is logged to stdout and execution of the handler stops. The current workaround to check whether all check_equal() assertions in a callback succeeded is to increment a success counter after the assertions (which only gets incremented if none exception is raised and stops execution). Then, outside the callback, the success counter can be used to check whether a callback executed successfully. One issue with the described workaround is that when an exception occurs, there is no way of telling which of the check_equal() statements caused the exception; moreover, there is no way of inspecting how the pieces of data that got compared in check_equal() differed (often a crucial clue when debugging what went wrong). Two fixes to this problem come to mind. The first involves having the callback function make event data accessible outside the callback and inspecting the event using check_equal() outside the callback. This solution still requires a counter in the callback to tell whether a callback was actually executed or if instead the call to perf_buffer_poll() timed out. The second fix entails wrapping all relevant check_equal() statements inside callback functions into try-catch blocks and manually logging AssertionErrors. While not as elegant in terms of design, this approach can be more pragmatic for more complex tests (e.g., ones involving multiple events, events of different types, or the order of events). The solution proposed here is to select the most pragmatic fix on a case-by-case basis: Tests in interface_usdt_net.py, interface_usdt_mempool.py and interface_usdt_validation.py have been refactored to use the first approach, while the second approach was chosen for interface_usdt_utxocache.py (partly to provide a reference for the second approach, but mainly because the utxocache tests are the most intricate tests, and refactoring them to use the first approach would negatively impact their readability). Lastly, interface_usdt_coinselection.py was kept unchanged because it does not use check_equal() statements inside callback functions.
1 parent 7f20197 commit 61f4b9b

File tree

4 files changed

+88
-90
lines changed

4 files changed

+88
-90
lines changed

test/functional/interface_usdt_mempool.py

+33-47
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ def added_test(self):
139139

140140
EXPECTED_ADDED_EVENTS = 1
141141
handled_added_events = 0
142+
event = None
142143

143144
self.log.info("Hooking into mempool:added tracepoint...")
144145
node = self.nodes[0]
@@ -147,32 +148,29 @@ def added_test(self):
147148
bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
148149

149150
def handle_added_event(_, data, __):
150-
nonlocal handled_added_events
151+
nonlocal event, handled_added_events
151152
event = bpf["added_events"].event(data)
152-
assert_equal(txid, bytes(event.hash)[::-1].hex())
153-
assert_equal(vsize, event.vsize)
154-
assert_equal(fee, event.fee)
155153
handled_added_events += 1
156154

157155
bpf["added_events"].open_perf_buffer(handle_added_event)
158156

159157
self.log.info("Sending transaction...")
160158
fee = Decimal(31200)
161159
tx = self.wallet.send_self_transfer(from_node=node, fee=fee / COIN)
162-
# expected data
163-
txid = tx["txid"]
164-
vsize = tx["tx"].get_vsize()
165160

166161
self.log.info("Polling buffer...")
167162
bpf.perf_buffer_poll(timeout=200)
168163

169164
self.log.info("Cleaning up mempool...")
170165
self.generate(node, 1)
171166

172-
bpf.cleanup()
173-
174167
self.log.info("Ensuring mempool:added event was handled successfully...")
175168
assert_equal(EXPECTED_ADDED_EVENTS, handled_added_events)
169+
assert_equal(bytes(event.hash)[::-1].hex(), tx["txid"])
170+
assert_equal(event.vsize, tx["tx"].get_vsize())
171+
assert_equal(event.fee, fee)
172+
173+
bpf.cleanup()
176174
self.generate(self.wallet, 1)
177175

178176
def removed_test(self):
@@ -181,6 +179,7 @@ def removed_test(self):
181179

182180
EXPECTED_REMOVED_EVENTS = 1
183181
handled_removed_events = 0
182+
event = None
184183

185184
self.log.info("Hooking into mempool:removed tracepoint...")
186185
node = self.nodes[0]
@@ -189,24 +188,16 @@ def removed_test(self):
189188
bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
190189

191190
def handle_removed_event(_, data, __):
192-
nonlocal handled_removed_events
191+
nonlocal event, handled_removed_events
193192
event = bpf["removed_events"].event(data)
194-
assert_equal(txid, bytes(event.hash)[::-1].hex())
195-
assert_equal(reason, event.reason.decode("UTF-8"))
196-
assert_equal(vsize, event.vsize)
197-
assert_equal(fee, event.fee)
198-
assert_equal(entry_time, event.entry_time)
199193
handled_removed_events += 1
200194

201195
bpf["removed_events"].open_perf_buffer(handle_removed_event)
202196

203197
self.log.info("Sending transaction...")
204198
fee = Decimal(31200)
205199
tx = self.wallet.send_self_transfer(from_node=node, fee=fee / COIN)
206-
# expected data
207200
txid = tx["txid"]
208-
reason = "expiry"
209-
vsize = tx["tx"].get_vsize()
210201

211202
self.log.info("Fast-forwarding time to mempool expiry...")
212203
entry_time = node.getmempoolentry(txid)["time"]
@@ -220,10 +211,15 @@ def handle_removed_event(_, data, __):
220211
self.log.info("Polling buffer...")
221212
bpf.perf_buffer_poll(timeout=200)
222213

223-
bpf.cleanup()
224-
225214
self.log.info("Ensuring mempool:removed event was handled successfully...")
226215
assert_equal(EXPECTED_REMOVED_EVENTS, handled_removed_events)
216+
assert_equal(bytes(event.hash)[::-1].hex(), txid)
217+
assert_equal(event.reason.decode("UTF-8"), "expiry")
218+
assert_equal(event.vsize, tx["tx"].get_vsize())
219+
assert_equal(event.fee, fee)
220+
assert_equal(event.entry_time, entry_time)
221+
222+
bpf.cleanup()
227223
self.generate(self.wallet, 1)
228224

229225
def replaced_test(self):
@@ -232,6 +228,7 @@ def replaced_test(self):
232228

233229
EXPECTED_REPLACED_EVENTS = 1
234230
handled_replaced_events = 0
231+
event = None
235232

236233
self.log.info("Hooking into mempool:replaced tracepoint...")
237234
node = self.nodes[0]
@@ -240,15 +237,8 @@ def replaced_test(self):
240237
bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
241238

242239
def handle_replaced_event(_, data, __):
243-
nonlocal handled_replaced_events
240+
nonlocal event, handled_replaced_events
244241
event = bpf["replaced_events"].event(data)
245-
assert_equal(replaced_txid, bytes(event.replaced_hash)[::-1].hex())
246-
assert_equal(replaced_vsize, event.replaced_vsize)
247-
assert_equal(replaced_fee, event.replaced_fee)
248-
assert_equal(replaced_entry_time, event.replaced_entry_time)
249-
assert_equal(replacement_txid, bytes(event.replacement_hash)[::-1].hex())
250-
assert_equal(replacement_vsize, event.replacement_vsize)
251-
assert_equal(replacement_fee, event.replacement_fee)
252242
handled_replaced_events += 1
253243

254244
bpf["replaced_events"].open_perf_buffer(handle_replaced_event)
@@ -267,21 +257,20 @@ def handle_replaced_event(_, data, __):
267257
from_node=node, utxo_to_spend=utxo, fee=replacement_fee / COIN
268258
)
269259

270-
# expected data
271-
replaced_txid = original_tx["txid"]
272-
replaced_vsize = original_tx["tx"].get_vsize()
273-
replaced_fee = original_fee
274-
replaced_entry_time = entry_time
275-
replacement_txid = replacement_tx["txid"]
276-
replacement_vsize = replacement_tx["tx"].get_vsize()
277-
278260
self.log.info("Polling buffer...")
279261
bpf.perf_buffer_poll(timeout=200)
280262

281-
bpf.cleanup()
282-
283263
self.log.info("Ensuring mempool:replaced event was handled successfully...")
284264
assert_equal(EXPECTED_REPLACED_EVENTS, handled_replaced_events)
265+
assert_equal(bytes(event.replaced_hash)[::-1].hex(), original_tx["txid"])
266+
assert_equal(event.replaced_vsize, original_tx["tx"].get_vsize())
267+
assert_equal(event.replaced_fee, original_fee)
268+
assert_equal(event.replaced_entry_time, entry_time)
269+
assert_equal(bytes(event.replacement_hash)[::-1].hex(), replacement_tx["txid"])
270+
assert_equal(event.replacement_vsize, replacement_tx["tx"].get_vsize())
271+
assert_equal(event.replacement_fee, replacement_fee)
272+
273+
bpf.cleanup()
285274
self.generate(self.wallet, 1)
286275

287276
def rejected_test(self):
@@ -290,6 +279,7 @@ def rejected_test(self):
290279

291280
EXPECTED_REJECTED_EVENTS = 1
292281
handled_rejected_events = 0
282+
event = None
293283

294284
self.log.info("Adding P2P connection...")
295285
node = self.nodes[0]
@@ -301,10 +291,8 @@ def rejected_test(self):
301291
bpf = BPF(text=MEMPOOL_TRACEPOINTS_PROGRAM, usdt_contexts=[ctx], debug=0)
302292

303293
def handle_rejected_event(_, data, __):
304-
nonlocal handled_rejected_events
294+
nonlocal event, handled_rejected_events
305295
event = bpf["rejected_events"].event(data)
306-
assert_equal(txid, bytes(event.hash)[::-1].hex())
307-
assert_equal(reason, event.reason.decode("UTF-8"))
308296
handled_rejected_events += 1
309297

310298
bpf["rejected_events"].open_perf_buffer(handle_rejected_event)
@@ -313,17 +301,15 @@ def handle_rejected_event(_, data, __):
313301
tx = self.wallet.create_self_transfer(fee_rate=Decimal(0))
314302
node.p2ps[0].send_txs_and_test([tx["tx"]], node, success=False)
315303

316-
# expected data
317-
txid = tx["tx"].hash
318-
reason = "min relay fee not met"
319-
320304
self.log.info("Polling buffer...")
321305
bpf.perf_buffer_poll(timeout=200)
322306

323-
bpf.cleanup()
324-
325307
self.log.info("Ensuring mempool:rejected event was handled successfully...")
326308
assert_equal(EXPECTED_REJECTED_EVENTS, handled_rejected_events)
309+
assert_equal(bytes(event.hash)[::-1].hex(), tx["tx"].hash)
310+
assert_equal(event.reason.decode("UTF-8"), "min relay fee not met")
311+
312+
bpf.cleanup()
327313
self.generate(self.wallet, 1)
328314

329315
def run_test(self):

test/functional/interface_usdt_net.py

+8-7
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,10 @@ def __repr__(self):
116116
fn_name="trace_outbound_message")
117117
bpf = BPF(text=net_tracepoints_program, usdt_contexts=[ctx], debug=0)
118118

119-
# The handle_* function is a ctypes callback function called from C. When
120-
# we assert in the handle_* function, the AssertError doesn't propagate
121-
# back to Python. The exception is ignored. We manually count and assert
122-
# that the handle_* functions succeeded.
123119
EXPECTED_INOUTBOUND_VERSION_MSG = 1
124120
checked_inbound_version_msg = 0
125121
checked_outbound_version_msg = 0
122+
events = []
126123

127124
def check_p2p_message(event, inbound):
128125
nonlocal checked_inbound_version_msg, checked_outbound_version_msg
@@ -142,12 +139,13 @@ def check_p2p_message(event, inbound):
142139
checked_outbound_version_msg += 1
143140

144141
def handle_inbound(_, data, __):
142+
nonlocal events
145143
event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents
146-
check_p2p_message(event, True)
144+
events.append((event, True))
147145

148146
def handle_outbound(_, data, __):
149147
event = ctypes.cast(data, ctypes.POINTER(P2PMessage)).contents
150-
check_p2p_message(event, False)
148+
events.append((event, False))
151149

152150
bpf["inbound_messages"].open_perf_buffer(handle_inbound)
153151
bpf["outbound_messages"].open_perf_buffer(handle_outbound)
@@ -158,12 +156,15 @@ def handle_outbound(_, data, __):
158156
bpf.perf_buffer_poll(timeout=200)
159157

160158
self.log.info(
161-
"check that we got both an inbound and outbound version message")
159+
"check receipt and content of in- and outbound version messages")
160+
for event, inbound in events:
161+
check_p2p_message(event, inbound)
162162
assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG,
163163
checked_inbound_version_msg)
164164
assert_equal(EXPECTED_INOUTBOUND_VERSION_MSG,
165165
checked_outbound_version_msg)
166166

167+
167168
bpf.cleanup()
168169

169170

test/functional/interface_usdt_utxocache.py

+30-19
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,16 @@ def handle_utxocache_uncache(_, data, __):
188188
nonlocal handle_uncache_succeeds
189189
event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents
190190
self.log.info(f"handle_utxocache_uncache(): {event}")
191-
assert_equal(block_1_coinbase_txid, bytes(event.txid[::-1]).hex())
192-
assert_equal(0, event.index) # prevout index
193-
assert_equal(EARLY_BLOCK_HEIGHT, event.height)
194-
assert_equal(50 * COIN, event.value)
195-
assert_equal(True, event.is_coinbase)
196-
197-
handle_uncache_succeeds += 1
191+
try:
192+
assert_equal(block_1_coinbase_txid, bytes(event.txid[::-1]).hex())
193+
assert_equal(0, event.index) # prevout index
194+
assert_equal(EARLY_BLOCK_HEIGHT, event.height)
195+
assert_equal(50 * COIN, event.value)
196+
assert_equal(True, event.is_coinbase)
197+
except AssertionError:
198+
self.log.exception("Assertion failed")
199+
else:
200+
handle_uncache_succeeds += 1
198201

199202
bpf["utxocache_uncache"].open_perf_buffer(handle_utxocache_uncache)
200203

@@ -260,24 +263,32 @@ def handle_utxocache_add(_, data, __):
260263
event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents
261264
self.log.info(f"handle_utxocache_add(): {event}")
262265
add = expected_utxocache_adds.pop(0)
263-
assert_equal(add["txid"], bytes(event.txid[::-1]).hex())
264-
assert_equal(add["index"], event.index)
265-
assert_equal(add["height"], event.height)
266-
assert_equal(add["value"], event.value)
267-
assert_equal(add["is_coinbase"], event.is_coinbase)
268-
handle_add_succeeds += 1
266+
try:
267+
assert_equal(add["txid"], bytes(event.txid[::-1]).hex())
268+
assert_equal(add["index"], event.index)
269+
assert_equal(add["height"], event.height)
270+
assert_equal(add["value"], event.value)
271+
assert_equal(add["is_coinbase"], event.is_coinbase)
272+
except AssertionError:
273+
self.log.exception("Assertion failed")
274+
else:
275+
handle_add_succeeds += 1
269276

270277
def handle_utxocache_spent(_, data, __):
271278
nonlocal handle_spent_succeeds
272279
event = ctypes.cast(data, ctypes.POINTER(UTXOCacheChange)).contents
273280
self.log.info(f"handle_utxocache_spent(): {event}")
274281
spent = expected_utxocache_spents.pop(0)
275-
assert_equal(spent["txid"], bytes(event.txid[::-1]).hex())
276-
assert_equal(spent["index"], event.index)
277-
assert_equal(spent["height"], event.height)
278-
assert_equal(spent["value"], event.value)
279-
assert_equal(spent["is_coinbase"], event.is_coinbase)
280-
handle_spent_succeeds += 1
282+
try:
283+
assert_equal(spent["txid"], bytes(event.txid[::-1]).hex())
284+
assert_equal(spent["index"], event.index)
285+
assert_equal(spent["height"], event.height)
286+
assert_equal(spent["value"], event.value)
287+
assert_equal(spent["is_coinbase"], event.is_coinbase)
288+
except AssertionError:
289+
self.log.exception("Assertion failed")
290+
else:
291+
handle_spent_succeeds += 1
281292

282293
bpf["utxocache_add"].open_perf_buffer(handle_utxocache_add)
283294
bpf["utxocache_spent"].open_perf_buffer(handle_utxocache_spent)

test/functional/interface_usdt_validation.py

+17-17
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,10 @@ def __repr__(self):
8585
self.sigops,
8686
self.duration)
8787

88-
# The handle_* function is a ctypes callback function called from C. When
89-
# we assert in the handle_* function, the AssertError doesn't propagate
90-
# back to Python. The exception is ignored. We manually count and assert
91-
# that the handle_* functions succeeded.
9288
BLOCKS_EXPECTED = 2
9389
blocks_checked = 0
9490
expected_blocks = dict()
91+
events = []
9592

9693
self.log.info("hook into the validation:block_connected tracepoint")
9794
ctx = USDT(pid=self.nodes[0].process.pid)
@@ -101,19 +98,10 @@ def __repr__(self):
10198
usdt_contexts=[ctx], debug=0)
10299

103100
def handle_blockconnected(_, data, __):
104-
nonlocal expected_blocks, blocks_checked
101+
nonlocal events, blocks_checked
105102
event = ctypes.cast(data, ctypes.POINTER(Block)).contents
106103
self.log.info(f"handle_blockconnected(): {event}")
107-
block_hash = bytes(event.hash[::-1]).hex()
108-
block = expected_blocks[block_hash]
109-
assert_equal(block["hash"], block_hash)
110-
assert_equal(block["height"], event.height)
111-
assert_equal(len(block["tx"]), event.transactions)
112-
assert_equal(len([tx["vin"] for tx in block["tx"]]), event.inputs)
113-
assert_equal(0, event.sigops) # no sigops in coinbase tx
114-
# only plausibility checks
115-
assert event.duration > 0
116-
del expected_blocks[block_hash]
104+
events.append(event)
117105
blocks_checked += 1
118106

119107
bpf["block_connected"].open_perf_buffer(
@@ -126,12 +114,24 @@ def handle_blockconnected(_, data, __):
126114
expected_blocks[block_hash] = self.nodes[0].getblock(block_hash, 2)
127115

128116
bpf.perf_buffer_poll(timeout=200)
129-
bpf.cleanup()
130117

131-
self.log.info(f"check that we traced {BLOCKS_EXPECTED} blocks")
118+
self.log.info(f"check that we correctly traced {BLOCKS_EXPECTED} blocks")
119+
for event in events:
120+
block_hash = bytes(event.hash[::-1]).hex()
121+
block = expected_blocks[block_hash]
122+
assert_equal(block["hash"], block_hash)
123+
assert_equal(block["height"], event.height)
124+
assert_equal(len(block["tx"]), event.transactions)
125+
assert_equal(len([tx["vin"] for tx in block["tx"]]), event.inputs)
126+
assert_equal(0, event.sigops) # no sigops in coinbase tx
127+
# only plausibility checks
128+
assert event.duration > 0
129+
del expected_blocks[block_hash]
132130
assert_equal(BLOCKS_EXPECTED, blocks_checked)
133131
assert_equal(0, len(expected_blocks))
134132

133+
bpf.cleanup()
134+
135135

136136
if __name__ == '__main__':
137137
ValidationTracepointTest().main()

0 commit comments

Comments
 (0)