Various changes needed to make network stack reset more reliable.

sdk/include/locks.hh

@@ -282,6 +282,39 @@ class LockGuard
 		try_lock(timeout);
 	}
 
+	/// Constructor, attempts to acquire the lock with a timeout, in steps
+	/// of a given duration, checking the `condition` function at every
+	/// iteration. If `condition` returns `true`, stop trying to acquire
+	/// the lock.
+	[[nodiscard]] explicit LockGuard(Lock    &lock,
+	                                 Timeout *timeout,
+	                                 Ticks    step,
+	                                 auto condition) requires(TryLockable<Lock>)
+	  : wrappedLock(&lock), isOwned(false)
+	{
+		do
+		{
+			Timeout t{std::min(step, timeout->remaining)};
+			try_lock(&t);
+			if (!t.elapsed)
+			{
+				// We failed to lock, likely because the lock
+				// is set for destruction.
+				break;
+			}
+			timeout->elapse(t.elapsed);
+		} while (!isOwned && timeout->may_block() && !condition());
+	}
+
+	/// Constructor, attempts to acquire the lock with a timeout, in steps
+	/// of a given duration.
+	[[nodiscard]] explicit LockGuard(Lock    &lock,
+	                                 Timeout *timeout,
+	                                 Ticks    step) requires(TryLockable<Lock>)
+	  : LockGuard(lock, timeout, step, []() { return false; })
+	{
+	}
+
 	/// Move constructor, transfers ownership of the lock.
 	[[nodiscard]] explicit LockGuard(LockGuard &&guard)
 	  : wrappedLock(guard.wrappedLock), isOwned(guard.isOwned)

sdk/lib/event_group/event_group.cc

@@ -8,6 +8,17 @@
 
 using Debug = ConditionalDebug<false, "Event groups library">;
 
+/**
+ * Length of a locking step for even group locks and futex words. When we
+ * attempt to grab an event group lock or wait on a futex, do so in steps of
+ * `EventGroupLockTimeoutStep`. That way, if the user of the event group
+ * crashes and we somehow do not have a pointer to the event group lock anymore
+ * (and thus cannot reset it), we have a guarantee that the blocking thread
+ * will become live again within a bounded amount of time, and crash as it is
+ * trying to re-acquire the lock if we free it through heap-free-all.
+ */
+static constexpr const Ticks EventGroupLockTimeoutStep = MS_TO_TICKS(500);
+
 struct EventWaiter
 {
 	std::atomic<uint32_t> bitsSeen;
@@ -74,7 +85,7 @@ int eventgroup_wait(Timeout    *timeout,
 	auto    &waiter = group->waiters[thread_id_get()];
 	uint32_t bitsSeen;
 	// Set up our state for the waiter with the lock held.
-	if (LockGuard g{group->lock, timeout})
+	if (LockGuard g{group->lock, timeout, EventGroupLockTimeoutStep})
 	{
 		bitsSeen = group->bits;
 		// If the condition holds, return immediately
@@ -98,15 +109,23 @@ int eventgroup_wait(Timeout    *timeout,
 	}
 	// If the condition didn't hold, wait for
 	Debug::log("Waiting on futex {} ({})", &waiter.bitsSeen, bitsSeen);
-	while (waiter.bitsSeen.wait(timeout, bitsSeen) != -ETIMEDOUT)
+	// Wait on the futex word in steps of `EventGroupLockTimeoutStep`
+	// ticks. See documentation for `EventGroupLockTimeoutStep` above.
+	do
 	{
-		bitsSeen = waiter.bitsSeen.load();
-		if (isTriggered(bitsSeen))
+		Timeout t{std::min(EventGroupLockTimeoutStep, timeout->remaining)};
+		while (waiter.bitsSeen.wait(&t, bitsSeen) != -ETIMEDOUT)
 		{
-			*outBits = bitsSeen;
-			return 0;
-		}
-	};
+			bitsSeen = waiter.bitsSeen.load();
+			if (isTriggered(bitsSeen))
+			{
+				timeout->elapse(t.elapsed);
+				*outBits = bitsSeen;
+				return 0;
+			}
+		};
+		timeout->elapse(t.elapsed);
+	} while (timeout->may_block());
 	waiter.bitsWanted = 0;
 	*outBits          = group->bits;
 	return -ETIMEDOUT;
@@ -117,7 +136,7 @@ int eventgroup_clear(Timeout    *timeout,
                      uint32_t   *outBits,
                      uint32_t    bitsToClear)
 {
-	if (LockGuard g{group->lock, timeout})
+	if (LockGuard g{group->lock, timeout, EventGroupLockTimeoutStep})
 	{
 		Debug::log(
 		  "Bits was {}, clearing with mask {}", group->bits, ~bitsToClear);
@@ -134,7 +153,7 @@ int eventgroup_set(Timeout    *timeout,
                    uint32_t   *outBits,
                    uint32_t    bitsToSet)
 {
-	if (LockGuard g{group->lock, timeout})
+	if (LockGuard g{group->lock, timeout, EventGroupLockTimeoutStep})
 	{
 		Debug::log("Bits was {}, setting {}", group->bits, bitsToSet);
 		group->bits |= bitsToSet;
@@ -197,6 +216,9 @@ int eventgroup_destroy_force(SObjStruct *heapCapability, EventGroup *group)
 
 int eventgroup_destroy(SObjStruct *heapCapability, EventGroup *group)
 {
-	group->lock.lock();
+	Timeout   unlimited{UnlimitedTimeout};
+	LockGuard g{group->lock, &unlimited, EventGroupLockTimeoutStep};
+	// The lock will be freed as part of `eventgroup_destroy_force`.
+	g.release();
 	return eventgroup_destroy_force(heapCapability, group);
 }

tests/locks-test.cc

@@ -65,6 +65,34 @@ namespace
 		}
 	}
 
+	/**
+	 * Test that lock guard conditions work as expected.
+	 */
+	template<typename Lock>
+	void test_lockguard_condition(Lock &lock)
+	{
+		// Duration of a step, in ticks.
+		static constexpr const Ticks Step = 10;
+
+		// Lock the lock
+		LockGuard g1{lock};
+
+		Timeout t{Step * 5};
+		// This will only ever return before the timeout if the
+		// condition becomes false, which should happen after two
+		// `Steps` (when `counter` is 2).
+		LockGuard g2{lock, &t, Step, [&]() { return (counter++ == 1); }};
+
+		TEST(t.elapsed != 0,
+		     "The lock guard returned before locking (0 tick elapsed)");
+		TEST(t.may_block(),
+		     "The lock guard ignored the condition and waited until timeout");
+		TEST(t.elapsed < 3 * Step,
+		     "The lock guard checked the condition too late ({} > {} ticks)",
+		     static_cast<int>(t.elapsed),
+		     static_cast<int>(3 * Step));
+	}
+
 	/**
 	 * Test that destructing a lock automatically wakes up all waiters,
 	 * failing them to acquire the lock.
@@ -405,6 +433,7 @@ int test_locks()
 	test_flaglock_unlock();
 	test_trylock(flagLock);
 	test_trylock(flagLockPriorityInherited);
+	test_lockguard_condition(flagLock);
 	test_destruct_lock_wake_up(flagLock);
 	test_destruct_lock_wake_up(flagLockPriorityInherited);
 	test_destruct_flag_lock_acquire();