From b7ffeb51aabefbed7c6a94fe9642122f5762791d Mon Sep 17 00:00:00 2001
From: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Date: Thu, 16 Jul 2020 13:34:24 +0200
Subject: [PATCH] cpu_sampler: avoid spinning on unavailable state

Fixes #3826
---
 repos/base-nova/include/cpu_thread/client.h   | 79 -------------------
 .../cpu_sampler/cpu_thread_component.cc       | 24 ++++--
 .../app/gdb_monitor/cpu_thread_component.cc   | 21 ++++-
 3 files changed, 36 insertions(+), 88 deletions(-)
 delete mode 100644 repos/base-nova/include/cpu_thread/client.h

diff --git a/repos/base-nova/include/cpu_thread/client.h b/repos/base-nova/include/cpu_thread/client.h
deleted file mode 100644
index 7f0444d557..0000000000
--- a/repos/base-nova/include/cpu_thread/client.h
+++ /dev/null
@@ -1,79 +0,0 @@
-/*
- * \brief  Client-side CPU thread interface
- * \author Norman Feske
- * \date   2016-05-10
- */
-
-/*
- * Copyright (C) 2016-2017 Genode Labs GmbH
- *
- * This file is part of the Genode OS framework, which is distributed
- * under the terms of the GNU Affero General Public License version 3.
- */
-
-#ifndef _INCLUDE__CPU_THREAD__CLIENT_H_
-#define _INCLUDE__CPU_THREAD__CLIENT_H_
-
-#include <cpu_thread/cpu_thread.h>
-#include <base/rpc_client.h>
-
-namespace Genode { struct Cpu_thread_client; }
-
-
-struct Genode::Cpu_thread_client : Rpc_client<Cpu_thread>
-{
-	explicit Cpu_thread_client(Thread_capability cap)
-	: Rpc_client<Cpu_thread>(cap) { }
-
-	Dataspace_capability utcb() override {
-		return call<Rpc_utcb>(); }
-
-	void start(addr_t ip, addr_t sp) override {
-		call<Rpc_start>(ip, sp); }
-
-	void pause() override {
-
-		for (;;) {
-
-			call<Rpc_pause>();
-
-			try {
-				/* check if the thread state is valid */
-				state();
-				/* the thread is paused */
-				return;
-			} catch (State_access_failed) {
-				/* the thread is (most likely) running on a different CPU */
-			}
-		}
-	}
-
-	void resume() override {
-		call<Rpc_resume>(); }
-
-	Thread_state state() override {
-		return call<Rpc_get_state>(); }
-
-	void state(Thread_state const &state) override {
-		call<Rpc_set_state>(state); }
-
-	void exception_sigh(Signal_context_capability handler) override {
-		call<Rpc_exception_sigh>(handler); }
-
-	void single_step(bool enabled) override {
-		call<Rpc_single_step>(enabled); }
-
-	void affinity(Affinity::Location location) override {
-		call<Rpc_affinity>(location); }
-
-	unsigned trace_control_index() override {
-		return call<Rpc_trace_control_index>(); }
-
-	Dataspace_capability trace_buffer() override {
-		return call<Rpc_trace_buffer>(); }
-
-	Dataspace_capability trace_policy() override {
-		return call<Rpc_trace_policy>(); }
-};
-
-#endif /* _INCLUDE__CPU_THREAD__CLIENT_H_ */
diff --git a/repos/gems/src/server/cpu_sampler/cpu_thread_component.cc b/repos/gems/src/server/cpu_sampler/cpu_thread_component.cc
index cfc82e4ba9..890fc128c6 100644
--- a/repos/gems/src/server/cpu_sampler/cpu_thread_component.cc
+++ b/repos/gems/src/server/cpu_sampler/cpu_thread_component.cc
@@ -77,24 +77,32 @@ void Cpu_sampler::Cpu_thread_component::take_sample()
 		return;
 	}
 
-	try {
+	enum { MAX_LOOP_CNT = 100 };
+	unsigned loop_cnt = 0;
+	for (; loop_cnt < MAX_LOOP_CNT; loop_cnt++) {
 
 		_parent_cpu_thread.pause();
 
-		Thread_state thread_state = _parent_cpu_thread.state();
+		try {
+
+			Thread_state thread_state = _parent_cpu_thread.state();
+
+			_sample_buf[_sample_buf_index++] = thread_state.ip;
+
+		} catch (State_access_failed) {
+			continue;
+		}
 
 		_parent_cpu_thread.resume();
 
-		_sample_buf[_sample_buf_index++] = thread_state.ip;
-
 		if (_sample_buf_index == SAMPLE_BUF_SIZE)
 			flush();
 
-	} catch (Cpu_thread::State_access_failed) {
-
-		Genode::log("thread state access failed");
-
+		break;
 	}
+
+	if (loop_cnt >= MAX_LOOP_CNT)
+		log("thread state access failed, ", _label.string());
 }
 
 
diff --git a/repos/ports/src/app/gdb_monitor/cpu_thread_component.cc b/repos/ports/src/app/gdb_monitor/cpu_thread_component.cc
index 5e378109e8..53812fac18 100644
--- a/repos/ports/src/app/gdb_monitor/cpu_thread_component.cc
+++ b/repos/ports/src/app/gdb_monitor/cpu_thread_component.cc
@@ -237,7 +237,26 @@ void Cpu_thread_component::start(addr_t ip, addr_t sp)
 
 void Cpu_thread_component::pause()
 {
-	_parent_cpu_thread.pause();
+	unsigned loop_cnt = 0;
+
+	/* required semantic for gdb is that thread is paused with valid state */
+	for (;;) {
+
+		_parent_cpu_thread.pause();
+
+		try {
+			/* check if the thread state is valid */
+			_parent_cpu_thread.state();
+			/* the thread is paused */
+			return;
+		} catch (State_access_failed) {
+			loop_cnt ++;
+
+			if (loop_cnt % 100 == 0)
+				Genode::warning("pausing thread failed ", loop_cnt,
+				                ". times, continue looping");
+		}
+	}
 }