Fix race condition between scheduler and debugger during all thread suspension.

arr committed 9 years ago

Commit bd141a71 parent a6da1dc5 files · +20 −33Browse files

Unified · Split

5 files

src/include/runtime.h +3 −3
src/vm/scheduler.cpp +5 −17
src/vm/scheduler.h +2 −2
src/vm/tool_dispatch.cpp +9 −10
src/vm/tool_dispatch.h +1 −1

@@ -827,11 +827,11 @@ namespace disvm · 826 unmodified lines
827	827	// There is no indication if this function failed or succeeded.
828	828	virtual void enqueue_blocked_thread(uint32_t thread_id) = 0;
829	829
	830	+ // Gets the number of system threads the scheduler is utilizing
	831	+ virtual std::size_t get_system_thread_count() const = 0;
	832	+
830	833	// Get all VM threads
831	834	virtual std::vector<std::shared_ptr<const vm_thread_t>> get_all_threads() const = 0;
832		−
833		− // Get runnable VM threads
834		− virtual std::vector<std::shared_ptr<const vm_thread_t>> get_runnable_threads() const = 0;
835	835	};
836	836
837	837	// VM thread scheduler interface

src/vm/scheduler.cpp+5−17

@@ -259,31 +259,19 @@ void default_scheduler_t::enqueue_blocked_thread(uint32_t thread_id) · 258 unmodified lines
259	259	}
260	260	}
261	261
262		−std::vector<std::shared_ptr<const vm_thread_t>> default_scheduler_t::get_all_threads() const
	262	+std::size_t default_scheduler_t::get_system_thread_count() const
263	263	{
264		− auto result = std::vector<std::shared_ptr<const vm_thread_t>>{};
265		−
266		− {
267		− std::lock_guard<std::mutex> lock{ _vm_threads_lock };
268		− for (auto &p : _all_vm_threads)
269		− result.push_back(p.second->vm_thread);
270		− }
271		−
272		− return result;
	264	+ return _worker_thread_count;
273	265	}
274	266
275		−std::vector<std::shared_ptr<const vm_thread_t>> default_scheduler_t::get_runnable_threads() const
	267	+std::vector<std::shared_ptr<const vm_thread_t>> default_scheduler_t::get_all_threads() const
276	268	{
277	269	auto result = std::vector<std::shared_ptr<const vm_thread_t>>{};
278	270
279	271	{
280	272	std::lock_guard<std::mutex> lock{ _vm_threads_lock };
281		− for (auto id : _runnable_vm_thread_ids)
282		− {
283		− auto iter = _all_vm_threads.find(id);
284		− assert(iter != _all_vm_threads.cend());
285		− result.push_back(iter->second->vm_thread);
286		− }
	273	+ for (auto &p : _all_vm_threads)
	274	+ result.push_back(p.second->vm_thread);
287	275	}
288	276
289	277	return result;

src/vm/scheduler.h+2−2

@@ -45,9 +45,9 @@ namespace disvm · 44 unmodified lines
45	45	public: // vm_scheduler_control_t
46	46	void enqueue_blocked_thread(uint32_t thread_id) override;
47	47
48		− std::vector<std::shared_ptr<const vm_thread_t>> get_all_threads() const override;
	48	+ std::size_t get_system_thread_count() const override;
49	49
50		− std::vector<std::shared_ptr<const vm_thread_t>> get_runnable_threads() const override;
	50	+ std::vector<std::shared_ptr<const vm_thread_t>> get_all_threads() const override;
51	51
52	52	private:
53	53	struct thread_instance_t final

src/vm/tool_dispatch.cpp+9−10

@@ -332,23 +332,22 @@ void vm_tool_dispatch_t::suspend_all_threads() · 331 unmodified lines
332	332
333	333	const auto &sched = _vm.get_scheduler_control();
334	334
335		− auto current = _threads_suspended_count.load();
336		− auto max = static_cast<uint32_t>(sched.get_runnable_threads().size());
	335	+ const auto sys_thread_count = static_cast<int32_t>(sched.get_system_thread_count());
	336	+ assert(sys_thread_count != 0);
337	337
338		− // Sleep and check until we determine all threads have been suspended
339		− // Minus 1 to exclude the current thread.
340		− while (max != 0 && current < (max - 1))
	338	+ // Sleep and check until we account for all system threads associated with the VM.
	339	+ auto suspended = _threads_suspended_count.load();
	340	+ while ((suspended + 1) < sys_thread_count)
341	341	{
342	342	std::this_thread::sleep_for(std::chrono::milliseconds{ 20 });
343		− current = _threads_suspended_count.load();
344		− max = static_cast<uint32_t>(sched.get_runnable_threads().size());
	343	+ suspended = _threads_suspended_count.load();
345	344	}
346	345
347		− // If the current count is ever equal to the max, then a non vm thread is
348		− // inside the tool dispatcher which may be okay, but it indicates a type of
	346	+ // If the suspended count is ever equal to the system thread count, then a non-vm thread
	347	+ // is inside the tool dispatcher which may be okay, but it indicates a type of
349	348	// non-invasive debugging that was not originally incorporated into this
350	349	// dispatcher design.
351		− assert(max == 0 \|\| current < max);
	350	+ assert(suspended < sys_thread_count);
352	351	}
353	352
354	353	void vm_tool_dispatch_t::resume_all_threads()

src/vm/tool_dispatch.h+1−1

@@ -82,7 +82,7 @@ namespace disvm · 81 unmodified lines
82	82	vm_t &_vm;
83	83
84	84	std::atomic_bool _threads_suspended;
85		− std::atomic<uint32_t> _threads_suspended_count;
	85	+ std::atomic<int32_t> _threads_suspended_count;
86	86	std::mutex _threads_suspend_lock;
87	87	std::condition_variable _threads_resume;
88	88	std::mutex _threads_resume_lock;