max / alloy
- Co-Authored-By
- Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+156 insertions,
-91 deletions
| @@ -1144,8 +1144,49 @@ | |||
| 1144 | 1144 | /// child, and Linux refuses to execute a file any process has open for | |
| 1145 | 1145 | /// writing. The window is microseconds wide and there is nothing to fix in | |
| 1146 | 1146 | /// `converse`, which is doing the ordinary thing. | |
| 1147 | + | /// | |
| 1148 | + | /// It closes half of the race and not all of it: the fork that loses can | |
| 1149 | + | /// come from any thread in the test binary. [`retrying`] covers the rest. | |
| 1147 | 1150 | static SCRIPTS: Mutex<()> = Mutex::new(()); | |
| 1148 | 1151 | ||
| 1152 | + | /// Linux refuses to execute a file that some process holds open for | |
| 1153 | + | /// writing, and this is the errno it says so with. | |
| 1154 | + | const ETXTBSY: i32 = 26; | |
| 1155 | + | ||
| 1156 | + | /// Run a scripted-helper conversation, re-attempting while the exec is | |
| 1157 | + | /// refused with `ETXTBSY`. | |
| 1158 | + | /// | |
| 1159 | + | /// The [`SCRIPTS`] lock serialises these tests against each other and | |
| 1160 | + | /// cannot cover this on its own: `Command::spawn` forks, and any thread in | |
| 1161 | + | /// the binary that forks between the script being written and its exec | |
| 1162 | + | /// holds a writable descriptor to that inode. A `cargo test` run that is | |
| 1163 | + | /// also compiling supplies those forks, which is when the failure shows up. | |
| 1164 | + | /// Test-only: production `converse` execs a setuid helper it never wrote, | |
| 1165 | + | /// so it cannot hit this. | |
| 1166 | + | /// | |
| 1167 | + | /// A refused exec runs nothing, so a retry repeats no side effect. | |
| 1168 | + | fn retrying(attempt: impl Fn() -> Result<()>) -> Result<()> { | |
| 1169 | + | for wait in [1u64, 2, 5, 10, 25, 50] { | |
| 1170 | + | match attempt() { | |
| 1171 | + | Err(err) if text_file_busy(&err) => { | |
| 1172 | + | std::thread::sleep(Duration::from_millis(wait)); | |
| 1173 | + | } | |
| 1174 | + | outcome => return outcome, | |
| 1175 | + | } | |
| 1176 | + | } | |
| 1177 | + | attempt() | |
| 1178 | + | } | |
| 1179 | + | ||
| 1180 | + | /// Whether anything in the error chain is `ETXTBSY`. | |
| 1181 | + | fn text_file_busy(err: &anyhow::Error) -> bool { | |
| 1182 | + | err.chain().any(|cause| { | |
| 1183 | + | cause | |
| 1184 | + | .downcast_ref::<std::io::Error>() | |
| 1185 | + | .and_then(std::io::Error::raw_os_error) | |
| 1186 | + | == Some(ETXTBSY) | |
| 1187 | + | }) | |
| 1188 | + | } | |
| 1189 | + | ||
| 1149 | 1190 | /// A stand-in for `polkit-agent-helper-1`: the same line protocol, written | |
| 1150 | 1191 | /// out as a shell script so a conversation can be tested end to end with no | |
| 1151 | 1192 | /// D-Bus, no polkit, and no setuid binary anywhere near it. This is what | |
| @@ -1238,15 +1279,18 @@ | |||
| 1238 | 1279 | fn the_typed_answer_reaches_the_helper_as_one_line() { | |
| 1239 | 1280 | let helper = scripted_helper("accepted", ASKS_ONCE); | |
| 1240 | 1281 | let asked = Arc::new(Mutex::new(Vec::new())); | |
| 1241 | - | let outcome = converse( | |
| 1242 | - | helper.path(), | |
| 1243 | - | "someone", | |
| 1244 | - | "cookie", | |
| 1245 | - | "an.action", | |
| 1246 | - | "polkit's sentence", | |
| 1247 | - | &Arc::new(AtomicBool::new(false)), | |
| 1248 | - | answering("letmein", &asked), | |
| 1249 | - | ); | |
| 1282 | + | let ask = answering("letmein", &asked); | |
| 1283 | + | let outcome = retrying(|| { | |
| 1284 | + | converse( | |
| 1285 | + | helper.path(), | |
| 1286 | + | "someone", | |
| 1287 | + | "cookie", | |
| 1288 | + | "an.action", | |
| 1289 | + | "polkit's sentence", | |
| 1290 | + | &Arc::new(AtomicBool::new(false)), | |
| 1291 | + | &ask, | |
| 1292 | + | ) | |
| 1293 | + | }); | |
| 1250 | 1294 | assert!(outcome.is_ok(), "{outcome:?}"); | |
| 1251 | 1295 | assert_eq!( | |
| 1252 | 1296 | asked.lock().expect("not poisoned").as_slice(), | |
| @@ -1258,15 +1302,18 @@ | |||
| 1258 | 1302 | fn a_wrong_answer_is_the_helpers_verdict_and_not_an_error_here() { | |
| 1259 | 1303 | let helper = scripted_helper("refused", ASKS_ONCE); | |
| 1260 | 1304 | let asked = Arc::new(Mutex::new(Vec::new())); | |
| 1261 | - | let outcome = converse( | |
| 1262 | - | helper.path(), | |
| 1263 | - | "someone", | |
| 1264 | - | "cookie", | |
| 1265 | - | "an.action", | |
| 1266 | - | "polkit's sentence", | |
| 1267 | - | &Arc::new(AtomicBool::new(false)), | |
| 1268 | - | answering("guess", &asked), | |
| 1269 | - | ); | |
| 1305 | + | let ask = answering("guess", &asked); | |
| 1306 | + | let outcome = retrying(|| { | |
| 1307 | + | converse( | |
| 1308 | + | helper.path(), | |
| 1309 | + | "someone", | |
| 1310 | + | "cookie", | |
| 1311 | + | "an.action", | |
| 1312 | + | "polkit's sentence", | |
| 1313 | + | &Arc::new(AtomicBool::new(false)), | |
| 1314 | + | &ask, | |
| 1315 | + | ) | |
| 1316 | + | }); | |
| 1270 | 1317 | assert_eq!(outcome.unwrap_err().to_string(), "not authorized"); | |
| 1271 | 1318 | } | |
| 1272 | 1319 | ||
| @@ -1277,15 +1324,18 @@ | |||
| 1277 | 1324 | fn an_answer_carrying_a_newline_is_refused_rather_than_written() { | |
| 1278 | 1325 | let helper = scripted_helper("newline", ASKS_ONCE); | |
| 1279 | 1326 | let asked = Arc::new(Mutex::new(Vec::new())); | |
| 1280 | - | let outcome = converse( | |
| 1281 | - | helper.path(), | |
| 1282 | - | "someone", | |
| 1283 | - | "cookie", | |
| 1284 | - | "an.action", | |
| 1285 | - | "polkit's sentence", | |
| 1286 | - | &Arc::new(AtomicBool::new(false)), | |
| 1287 | - | answering("letmein\nSUCCESS", &asked), | |
| 1288 | - | ); | |
| 1327 | + | let ask = answering("letmein\nSUCCESS", &asked); | |
| 1328 | + | let outcome = retrying(|| { | |
| 1329 | + | converse( | |
| 1330 | + | helper.path(), | |
| 1331 | + | "someone", | |
| 1332 | + | "cookie", | |
| 1333 | + | "an.action", | |
| 1334 | + | "polkit's sentence", | |
| 1335 | + | &Arc::new(AtomicBool::new(false)), | |
| 1336 | + | &ask, | |
| 1337 | + | ) | |
| 1338 | + | }); | |
| 1289 | 1339 | assert_eq!( | |
| 1290 | 1340 | outcome.unwrap_err().to_string(), | |
| 1291 | 1341 | "an answer cannot contain a newline", | |
| @@ -1298,15 +1348,18 @@ | |||
| 1298 | 1348 | fn a_withdrawn_conversation_asks_nothing() { | |
| 1299 | 1349 | let helper = scripted_helper("withdrawn", ASKS_ONCE); | |
| 1300 | 1350 | let asked = Arc::new(Mutex::new(Vec::new())); | |
| 1301 | - | let outcome = converse( | |
| 1302 | - | helper.path(), | |
| 1303 | - | "someone", | |
| 1304 | - | "cookie", | |
| 1305 | - | "an.action", | |
| 1306 | - | "polkit's sentence", | |
| 1307 | - | &Arc::new(AtomicBool::new(true)), | |
| 1308 | - | answering("letmein", &asked), | |
| 1309 | - | ); | |
| 1351 | + | let ask = answering("letmein", &asked); | |
| 1352 | + | let outcome = retrying(|| { | |
| 1353 | + | converse( | |
| 1354 | + | helper.path(), | |
| 1355 | + | "someone", | |
| 1356 | + | "cookie", | |
| 1357 | + | "an.action", | |
| 1358 | + | "polkit's sentence", | |
| 1359 | + | &Arc::new(AtomicBool::new(true)), | |
| 1360 | + | &ask, | |
| 1361 | + | ) | |
| 1362 | + | }); | |
| 1310 | 1363 | assert_eq!(outcome.unwrap_err().to_string(), "withdrawn"); | |
| 1311 | 1364 | assert!( | |
| 1312 | 1365 | asked.lock().expect("not poisoned").is_empty(), | |
| @@ -1327,15 +1380,18 @@ | |||
| 1327 | 1380 | let (seen, prompts) = sync_channel(1); | |
| 1328 | 1381 | let watching = Arc::clone(&withdrawn); | |
| 1329 | 1382 | let conversing = std::thread::spawn(move || { | |
| 1330 | - | converse( | |
| 1331 | - | &path, | |
| 1332 | - | "someone", | |
| 1333 | - | "cookie", | |
| 1334 | - | "an.action", | |
| 1335 | - | "polkit's sentence", | |
| 1336 | - | &watching, | |
| 1337 | - | move |prompt| seen.send(prompt).map_err(|_| anyhow!("nobody listening")), | |
| 1338 | - | ) | |
| 1383 | + | let ask = move |prompt| seen.send(prompt).map_err(|_| anyhow!("nobody listening")); | |
| 1384 | + | retrying(|| { | |
| 1385 | + | converse( | |
| 1386 | + | &path, | |
| 1387 | + | "someone", | |
| 1388 | + | "cookie", | |
| 1389 | + | "an.action", | |
| 1390 | + | "polkit's sentence", | |
| 1391 | + | &watching, | |
| 1392 | + | &ask, | |
| 1393 | + | ) | |
| 1394 | + | }) | |
| 1339 | 1395 | }); | |
| 1340 | 1396 | ||
| 1341 | 1397 | let prompt = prompts | |
| @@ -1379,21 +1435,24 @@ | |||
| 1379 | 1435 | let withdrawn = Arc::new(AtomicBool::new(false)); | |
| 1380 | 1436 | ||
| 1381 | 1437 | let cancelling = Arc::clone(&withdrawn); | |
| 1382 | - | let outcome = converse( | |
| 1383 | - | helper.path(), | |
| 1384 | - | "someone", | |
| 1385 | - | "cookie", | |
| 1386 | - | "an.action", | |
| 1387 | - | "polkit's sentence", | |
| 1388 | - | &withdrawn, | |
| 1389 | - | move |prompt| { | |
| 1390 | - | // polkit withdraws while the question is up, and the answer is | |
| 1391 | - | // sent anyway: the keypress and the withdrawal crossed. | |
| 1392 | - | cancelling.store(true, Ordering::Relaxed); | |
| 1393 | - | std::thread::spawn(move || prompt.answer(Secret::new("letmein"))); | |
| 1394 | - | Ok(()) | |
| 1395 | - | }, | |
| 1396 | - | ); | |
| 1438 | + | let ask = move |prompt: Prompt| { | |
| 1439 | + | // polkit withdraws while the question is up, and the answer is | |
| 1440 | + | // sent anyway: the keypress and the withdrawal crossed. | |
| 1441 | + | cancelling.store(true, Ordering::Relaxed); | |
| 1442 | + | std::thread::spawn(move || prompt.answer(Secret::new("letmein"))); | |
| 1443 | + | Ok(()) | |
| 1444 | + | }; | |
| 1445 | + | let outcome = retrying(|| { | |
| 1446 | + | converse( | |
| 1447 | + | helper.path(), | |
| 1448 | + | "someone", | |
| 1449 | + | "cookie", | |
| 1450 | + | "an.action", | |
| 1451 | + | "polkit's sentence", | |
| 1452 | + | &withdrawn, | |
| 1453 | + | &ask, | |
| 1454 | + | ) | |
| 1455 | + | }); | |
| 1397 | 1456 | ||
| 1398 | 1457 | assert_eq!(outcome.unwrap_err().to_string(), "withdrawn"); | |
| 1399 | 1458 | assert!( | |
| @@ -1476,27 +1535,30 @@ | |||
| 1476 | 1535 | let asked = Arc::new(Mutex::new(Vec::new())); | |
| 1477 | 1536 | let seen = Arc::new(Mutex::new(Vec::new())); | |
| 1478 | 1537 | let messages = Arc::clone(&seen); | |
| 1479 | - | let outcome = converse( | |
| 1480 | - | helper.path(), | |
| 1481 | - | "someone", | |
| 1482 | - | "cookie", | |
| 1483 | - | // A bidi override in the one string that was not being stripped. | |
| 1484 | - | // The action is what the log pane names twice, and reversing the | |
| 1485 | - | // rest of a line is how a name reads as one action and is another. | |
| 1486 | - | "an.\u{202e}action", | |
| 1487 | - | "polkit's \u{1b}[2Jsentence", | |
| 1488 | - | &Arc::new(AtomicBool::new(false)), | |
| 1489 | - | { | |
| 1490 | - | let answer = answering("letmein", &asked); | |
| 1491 | - | move |prompt: Prompt| { | |
| 1492 | - | messages | |
| 1493 | - | .lock() | |
| 1494 | - | .expect("not poisoned") | |
| 1495 | - | .push(format!("{}|{}", prompt.action_id, prompt.message)); | |
| 1496 | - | answer(prompt) | |
| 1497 | - | } | |
| 1498 | - | }, | |
| 1499 | - | ); | |
| 1538 | + | let ask = { | |
| 1539 | + | let answer = answering("letmein", &asked); | |
| 1540 | + | move |prompt: Prompt| { | |
| 1541 | + | messages | |
| 1542 | + | .lock() | |
| 1543 | + | .expect("not poisoned") | |
| 1544 | + | .push(format!("{}|{}", prompt.action_id, prompt.message)); | |
| 1545 | + | answer(prompt) | |
| 1546 | + | } | |
| 1547 | + | }; | |
| 1548 | + | let outcome = retrying(|| { | |
| 1549 | + | converse( | |
| 1550 | + | helper.path(), | |
| 1551 | + | "someone", | |
| 1552 | + | "cookie", | |
| 1553 | + | // A bidi override in the one string that was not being stripped. | |
| 1554 | + | // The action is what the log pane names twice, and reversing the | |
| 1555 | + | // rest of a line is how a name reads as one action and is another. | |
| 1556 | + | "an.\u{202e}action", | |
| 1557 | + | "polkit's \u{1b}[2Jsentence", | |
| 1558 | + | &Arc::new(AtomicBool::new(false)), | |
| 1559 | + | &ask, | |
| 1560 | + | ) | |
| 1561 | + | }); | |
| 1500 | 1562 | assert!(outcome.is_ok(), "{outcome:?}"); | |
| 1501 | 1563 | assert_eq!( | |
| 1502 | 1564 | asked.lock().expect("not poisoned").as_slice(), | |
| @@ -1530,17 +1592,20 @@ | |||
| 1530 | 1592 | let helper = scripted_helper("undeliverable", SLEEPS); | |
| 1531 | 1593 | let before = descendants(); | |
| 1532 | 1594 | ||
| 1533 | - | let outcome = converse( | |
| 1534 | - | helper.path(), | |
| 1535 | - | "someone", | |
| 1536 | - | "cookie", | |
| 1537 | - | "an.action", | |
| 1538 | - | "a message", | |
| 1539 | - | &Arc::new(AtomicBool::new(false)), | |
| 1540 | - | // Exactly what `begin_authentication` passes when the receiver has | |
| 1595 | + | let ask = // Exactly what `begin_authentication` passes when the receiver has | |
| 1541 | 1596 | // gone: the console stopped listening. | |
| 1542 | - | |_prompt| bail!("the console stopped listening"), | |
| 1543 | - | ); | |
| 1597 | + | |_prompt| bail!("the console stopped listening"); | |
| 1598 | + | let outcome = retrying(|| { | |
| 1599 | + | converse( | |
| 1600 | + | helper.path(), | |
| 1601 | + | "someone", | |
| 1602 | + | "cookie", | |
| 1603 | + | "an.action", | |
| 1604 | + | "a message", | |
| 1605 | + | &Arc::new(AtomicBool::new(false)), | |
| 1606 | + | &ask, | |
| 1607 | + | ) | |
| 1608 | + | }); | |
| 1544 | 1609 | assert!( | |
| 1545 | 1610 | outcome.is_err(), | |
| 1546 | 1611 | "the conversation should not have succeeded" |