strange "R6010-abort() has been called" error

[Eric Jones]

I've got a console program which essentially automates a large number of commandline apps, and to implement ghetto threading (because getting into real threading is a bit above my head), I'm using system() to execute psexec (in detached mode), calling the app, within a while loop. obviously, this can cause problems due to slower programs stacking up. during the initial write, this was not a problem (since the program execution would slow down when all cores were saturated), but now that I'm using the -belownormal switch in psexec, the program will continue to call new instances and it can cause crashes.

to control this, I've used pslist in php for a similar program, and was using that for a while, but due to the limits of my c++ knowledge, using pipes isn't really useful, and so I made a little function that counts the number of active threads with a given name, and just stay in a while loop as long as the number of active threads is too high. unfortunately, it makes the program crash with the "R6010-abort() has been called" error. to be clear, it worked without these strange crashes before I implemented the function to keep track of how many threads were running.

applicable section of code is here:

unsigned int ci(std::wstring name){
	DWORD procs[1024],sizeret,sizeret1;
	TCHAR procnameo[MAX_PATH];
	HANDLE hproc;
	HMODULE hmod;
	unsigned int loop,count=0;
	std::wstring procname;
	EnumProcesses(procs,sizeof(procs),&sizeret);
	for(loop=0;loop<sizeret/sizeof(DWORD);loop++){
		hproc=OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ,FALSE,procs[loop]);
		EnumProcessModules(hproc,&hmod,sizeof(hmod),&sizeret1);
		GetModuleBaseName(hproc,hmod,procnameo,sizeof(procnameo)/sizeof(TCHAR));
		procname.assign(procnameo);
		if(procname.find(name)<=1000000)count++;
		CloseHandle(hproc);
	}return count;
}

and the function is called via: search.assign(L"convert");while(ci(search)>16){Sleep(100);}

yes, I realize that my code is horrible.

anyways, any help would be much appreciated.

please note that when debugging, it does not crash at the same place, so I can't figure out the problem from that.

EDIT:

cleaned it up to make it more readable. when i say it doesn't crash at the same place, i mean it crashes like 300 lines later, and with a completely different error.

[kbw]

Learning threading has got to be easier than what you're doing.

You really should write one statement per line. It's conventional, readable and the debugger works on lines, not statements.

[Eric Jones]

fixed the code to be more readable. the reason this was easier than learning threading is that i was able to find a tutorial with a code snippet that printed out all running processes, and simply adapt it to this. yes, I know that this means I don't *learn* as much, but I'm not a student or (obviously) professional programmer; I just occasionally write a little code on the side when I need something I can't find anywhere.

I *am* looking into threading, at your suggestion, but after 6 hours, I haven't really gotten anywhere. I've opened a thread regarding that, but still consider this one open. I don't care how it gets done, all I really want is to be able to run 8 concurrent processes, wait for one to finish, then add another one, till i've processed everything I need to run.

as mentioned before, this is essentially a port (and slight expansion) of a php project I wrote earlier, and in php it was simple to accomplish because exec() automatically returns the output and not just the return code. i could thus use pslist to accomplish my goal (by searching the returned string for instances of the program name). I looked into trying to do the same in c++, but it was WAY over my head, and the only other option -- piping the output to a file thru dos, then reading the file in c++ and parsing it -- adds a large amount of overhead when executed every tenth of a second or so.

[clanmjc]

I suggest Boost::threads, they are very easy to use, open source and you can probably get a simple multithreaded example running today. You can also create thread pools based on the available hardware running the software through...

This is just a suggestion and can be omitted if you know you only want x number of threads.
boost::thread::hardware_concurrency() * 2; //where 2 is the number of thread you want to spawn per core

Starting a thread...

//Start: some method in MyClass, implementation below.
//m_mythread: member variable of MyClass of type boost::thread.
//WokerThread: a member function of MyClass, has the implementation for the work to do.

bool MyClass::Start()
{
	m_mythread = boost::thread(boost::bind(&MyClass::WokerThread, this));
	return true;
}

//This is all that is required to start a thread using boost. 

Stopping the thread (called from the deconstructor for the class in my case).

bool MyClass::Stop()
{
   if(m_mythread .joinable())
  {
     m_mythread .join();
     return true;
  }
  return false;
}

void MyClass::WokerThread()
{
   //Do stuff...
}

_{I've left out condition state variables and what not to make this easier to read, and you do not need state variables depending on your work and implementation.}

[Eric Jones]

i may just be missing something (like an include, or that I'm supposed to replace it with something), but after copypasta:

#include <iostream>
#include <string>
#include <boost/thread.hpp>
using namespace std;
#include <stdio.h>
#include <stdlib.h>
#include <Windows.h>

void MyClass::Worker()
{
   //Do stuff...
}

bool MyClass::Start()
{
	m_mythread = boost::thread(boost::bind(&MyClass::Worker, this));
	return true;
}

bool MyClass::Stop()
{
   if(m_mythread .joinable())
  {
     m_mythread .join();
     return true;
  }
  return false;
}

int main(void){

	cout << "Press Enter to Continue";
cin.ignore(999,'\n');
    return 0;
}

i get a bunch of compiler errors, including "1>d:\imagehack\testing\imagehack.cpp(9): error C2653: 'MyClass' : is not a class or namespace name" and "1>d:\imagehack\testing\imagehack.cpp(16): error C2065: 'm_mythread' : undeclared identifier" and "1>d:\imagehack\testing\imagehack.cpp(16): error C2355: 'this' : can only be referenced inside non-static member functions"

keep in mind that while i get some of this (like joinable and join), some is jargon (like bind) -- I typically find some code, copypasta it, then twiddle with it until I can understand what's going on.

i really appreciate the help, btw.

[clanmjc]

Yeah ok, it wasn't meant for copy paste, I thought you knew about classes / objects etc. No worries, I created you a class for executing "tasks" using boost::threads. As it stands a task is just a string, you could use it for a file name etc. You can change it to whatever you need this is just a very simple starting point, who knows you may be able to copy your code and paste into the TaskThread and do your stuff. I don't know what version of boost you are running however here is the code.

I also didn't implement copy move ctor, copy move assignment, etc...

Execute.h

#pragma once
#include <boost/thread.hpp>
#include <string>
class Execute
{
public:
	Execute();
	virtual ~Execute();
	void TaskThread();
	bool Start(const std::string &task);
	bool Stop();
	bool IsBusy()const;
protected:
private:
	volatile bool m_busy;
	std::string m_task;
	boost::thread m_thread;
};

Execute.cpp

#include "Execute.h"
#include <iostream>
#include <Windows.h>

Execute::Execute() : m_busy(false)
{
}

Execute::~Execute()
{
	Stop();
}

bool Execute::Stop()
{
   if(m_thread.joinable())
  {
     m_thread.join();
     return true;
  }
   return false;
}

bool Execute::Start(const std::string &task)
{
	m_busy = true;
	m_task = task;
	
	m_thread = boost::thread(boost::bind(&Execute::TaskThread, this));
	Sleep(500);
	return true;
}

void Execute::TaskThread()
{
   //This is where you need to do your work, open file, call another batch, job w/e
  //close file etc.
	std::cout << "Inside thread " << m_task << std::endl;
	m_busy = false; //This needs to be the last thing in here
	return;
}

bool Execute::IsBusy()const
{
	//Sleep(500); can put a sleep here if you don't want to constantly poll
	//A better option would be to use condition variables for shutdown etc. but I'm keeping this simple for you.
	return m_busy;
}

_{There is virtually no synchronization here.} To keep this as simple as I can I left it at this, I'll try to answer your questions time permitting. Things like I put the stop in the deconstructor, I did it there so you could use my example as is. You could not call it there, then call stop yourself...

I left the cout in there so you could mess around with it.

int main()
{
   { //Scope
	Execute exe, exe2, exe3;
	exe.Start("task1");
	exe2.Start("task2");
	exe3.Start("task3");
   } //Deconstruction
   return 0;
}

[Eric Jones]

upon initial copypasta, this doesn't seem to provide a method of limiting execution to an arbitrary number of threads at a time. I can either make a bunch of exe's, and launch them all concurrently (which would be silly, since it's that exact behaviour that I'm using threads to avoid ;P), or run them in batches, ie:

int main(void){
	unsigned int count=10,loop;
	 for(loop=0;loop<count;loop++){ //Scope
		Execute exe1, exe2, exe3,exe4,exe5,exe6,exe7,exe8;
		if(loop<count)exe1.Start("task1");loop++;
		if(loop<count)exe2.Start("task2");loop++;
		if(loop<count)exe3.Start("task3");loop++;
		if(loop<count)exe4.Start("task4");loop++;
		if(loop<count)exe5.Start("task5");loop++;
		if(loop<count)exe6.Start("task6");loop++;
		if(loop<count)exe7.Start("task7");loop++;
		if(loop<count)exe8.Start("task8");
	} //Deconstruction
	cout << "Press Enter to Continue";
cin.ignore(999,'\n');
    return 0;
}

in which, it will execute 8 threads, wait till they're all done, then execute the other 2. what I'm trying for is something where (in the case of 10 executions desired, as above) it will launch the initial 8, then as each one finishes, launch another until all 10 have been launched, ie:

enter loop->launch 1-8->4 finishes->launch 9->2 finishes->launch 10-> 1,3,5,6,7,8,9,10 finish->exit loop

ideally, the best method would be to keep track of how many threads are running and use that to control it. if i only needed to run 10 at a time, it wouldn't be a problem, but it can easily balloon over 1000 in my program, so it becomes a huge bottleneck.

i could of course use duct tape and do something like writing a "lock" file while the thread is running, then deleting the lock when it's done, and simply counting the number of lock files to tell how many threads are running... but this will add unnecessary overhead.

you may actually have this mechanism somewhere in the code, but I can't see it.

again, much thanks for the help!

EDIT:

I figured out how to accomplish what i want -- something like this:

	string command("");
	bool one=false,two=false,three=false,four=false,five=false,six=false,seven=false,eight=false;
	unsigned int count=10,loop;
	loop=0;while(loop<count){
		command.assign("psexec -accepteula -belownormal optipng -zc1-3 -zm1-3 -zs1-3 -f1-3 -i0 -zw 32k \"D:/testing/input/Gantz_v01c01p010{t.png-task1.png\" >nul 2>&1");
		if(loop<count && one==false){one=true;boost::thread thread1(worker,&one,command);loop++;}
		command.assign("psexec -accepteula -belownormal optipng -zc1-4 -zm1-4 -zs1-3 -f1-4 -i0 -zw 32k \"D:/testing/input/Gantz_v01c01p010{t.png-task1.png\" >nul 2>&1");
		if(loop<count && two==false){two=true;boost::thread thread2(worker,&two,command);loop++;}
		command.assign("psexec -accepteula -belownormal optipng -zc1-5 -zm1-5 -zs0-3 -f1-5 -i0 -zw 32k \"D:/testing/input/Gantz_v01c01p010{t.png-task1.png\" >nul 2>&1");
		if(loop<count && three==false){three=true;boost::thread thread3(worker,&three,command);loop++;}
		command.assign("psexec -accepteula -belownormal optipng -zc1-6 -zm1-6 -zs0-3 -f0-5 -i0 -zw 32k \"D:/testing/input/Gantz_v01c01p010{t.png-task1.png\" >nul 2>&1");
		if(loop<count && four==false){four=true;boost::thread thread4(worker,&four,command);loop++;}
//		if(loop<count && five==false){five=true;boost::thread thread5(worker,&five,command);loop++;}
//		if(loop<count && six==false){six=true;boost::thread thread6(worker,&six,command);loop++;}
//		if(loop<count && seven==false){seven=true;boost::thread thread7(worker,&seven,command);loop++;}
//		if(loop<count && eight==false){eight=true;boost::thread thread8(worker,&eight,command);loop++;}
	}

as you can see, I'm passing the pointer to the function, and right before the function exits, it flips the bool back to false -- essentially, it's a lock file, but in ram (bool, even), so a minimal performance hit. unfortunately, it means that the number of concurrent threads is essentially hardcoded, but I can add hardware concurrency to the if statement to keep it from executing more threads than the cpu can handle. so really only the maximum number of threads is hardcoded.

much thanks clanmjc, i couldn't have done it without you!

[clanmjc]

Great glad I could help. I whipped this up for you as well...

Here you go.. replace your Execute.h and .cpp with these... I've used depreciated things in main. This will allow you to.

1) Create a vector of commands.
2) Add those commands to a task queue
3) concurrently process the queue of tasks

The thread pool is hardcoded but can be easily changed. I've also included a max thread count that based on the hardware, you can add some code to use whichever is greater but right now it always uses 8.

#pragma once
#include <boost/thread.hpp>
#include <string>
#include <queue>

typedef boost::shared_ptr<boost::thread> SharedThreadPtr;

class Execute
{
public:
	static Execute& GetExecuter();
	virtual ~Execute();
	void TaskThread();
	bool Start();
	bool Stop();
	void AddTask(const std::string &task);
	bool IsTaskQueueEmpty();
protected:
private:
	Execute(int);
	Execute(const Execute& );
	Execute& operator=(const Execute& );
	
	int threadcount;
	int maxthreadcount;
	std::queue<std::string> m_taskqueue;

	std::vector<SharedThreadPtr> m_threads;
	
	boost::mutex mut_TaskQueue_;
};

.cpp

#include "Execute.h"
#include <iostream>
#include <Windows.h>

Execute::Execute(int mincount) : threadcount(mincount)
{
	maxthreadcount = boost::thread::hardware_concurrency() * 2;
}

Execute::~Execute()
{
	
}

bool Execute::Stop()
{
	for(size_t i = 0; i < m_threads.size(); ++i)
		if(m_threads[i]->joinable())
			m_threads[i]->join();

	m_threads.clear();
	return true;
}

bool Execute::Start()
{
	
	for(int i = 0; i < threadcount; ++i)
	{
		m_threads.push_back(SharedThreadPtr(new boost::thread(boost::bind(&Execute::TaskThread, this))));
	}
	return true;
}

void Execute::TaskThread()
{
	while(1)
	{
		std::string taskToProcess;
		{
			boost::lock_guard<boost::mutex> lock(mut_TaskQueue_);
			if(!m_taskqueue.empty())
			{
				taskToProcess = m_taskqueue.front();
				m_taskqueue.pop();
			}else
				break;
		}
		//Process task here...
		std::cout << "Inside thread " << taskToProcess << std::endl;
	}
	
	
}

Execute& Execute::GetExecuter()
{
	static Execute instance(8);
	return instance;
}

void Execute::AddTask(const std::string &task)
{
	boost::lock_guard<boost::mutex> lock(mut_TaskQueue_);
	m_taskqueue.push(task);
}

bool Execute::IsTaskQueueEmpty()
{
	Sleep(100);
	boost::lock_guard<boost::mutex> lock(mut_TaskQueue_);
	return m_taskqueue.empty();
}

main

#include "Execute.h"
#include "boost/lexical_cast.hpp"

std::vector<std::string> MakeFakeTasks();

int _tmain(int argc, _TCHAR* argv[])
{
	std::vector<std::string> tasks = MakeFakeTasks();
	
	Execute& exe = Execute::GetExecuter();
	for(std::vector<std::string>::iterator it = tasks.begin(); it != tasks.end(); ++it)
	{
		exe.AddTask(*it);
	}
	exe.Start();
	while(!exe.IsTaskQueueEmpty())
	{

	}
	exe.Stop();
	return 0;
}

std::vector<std::string> MakeFakeTasks()
{
	std::vector<std::string> fake;
	for(int i = 0; i < 10000; ++i)
	{
		std::string temp("task ");
		char buff[100];
		temp+=itoa(i,buff,10);
		fake.push_back(temp);
	}
	return fake;
}