COMMITTED: UCS crash fix

[Zothen]

Okay, I think it makes sense to post my fix here, since my UCS isnt crashing anymore.

I think I found the bug in UCS that is causing a crash.

Debugging showed up a halt in DBCore::Open() after a call of mysql_real_connect() using an uninitialized mysql struct.

The struct gets uninitialized whenever theres a connection error of some sort, because mysql.close() makes it invalid. Without another call to mysql.init() all following sql connects/commands will fail, resulting in a crash.

So we just need to enter a single line of code... :

(May need some more testing, but mine is running stable now for 20 hours.)

bool DBcore::Open(int32* errnum, char* errbuf) {
	if (errbuf)
		errbuf[0] = 0;
	LockMutex lock(&MDatabase);
	if (GetStatus() == Connected)
		return true;
	if (GetStatus() == Error)
	{
		mysql_close(&mysql);          // <- Makes struct 'mysql' invalid!
		mysql_init(&mysql);        //   <- Initialize structure again
	}
	if (!pHost)
		return false;
	/*
	Quagmire - added CLIENT_FOUND_ROWS flag to the connect
	otherwise DB update calls would say 0 rows affected when the value already equalled
	what the function was tring to set it to, therefore the function would think it failed 
	*/
	int32 flags = CLIENT_FOUND_ROWS;
	if (pCompress)
		flags |= CLIENT_COMPRESS;
	if (pSSL)
		flags |= CLIENT_SSL;
	// crashed here, because 'mysql' wasnt valid after an error followed by mysql.close() 
	if (mysql_real_connect(&mysql, pHost, pUser, pPassword, pDatabase, pPort, 0, flags)) { 
		pStatus = Connected;
		return true;
	}
	else {
		if (errnum)
			*errnum = mysql_errno(&mysql);
		if (errbuf)
			snprintf(errbuf, MYSQL_ERRMSG_SIZE, "#%i: %s", mysql_errno(&mysql), mysql_error(&mysql));
		pStatus = Error;
		return false;
	}
}

[trevius]

Sounds good to me. I will give it a shot and commit it when I have a few minutes. Thanks for the submission!

[Zothen]

Sure thing, happy to contribute to this project.

[sorvani]

My UCS has been running for 24+ hours now after this change. (Windows server)

[Secrets]

Shouldn't you just return false, then initialize the mysql connection after that if it is false? That makes more sense to me.

[Zothen]

I guess youre right regarding the return false. With my quick and dirty fix you may not get any database error messages again (some may think thats a good thing though LOL).
But on the other hand that way its possible that the UCS will just quit clean after it encounters a database error. I am not sure about that.

I would leave init where it is, since its the only place where the mysql struct gets closed (except in the destructor), so theres no reason to call it elsewhere.

[joligario]

I think what he may be saying is that instead of closing mysql, return false instead? Then the connection wouldn't need to be re-initialized.

[sorvani]

came down to my office and looked at the PC running my test server and noticed that UCS crashed over night. :(

[Zothen]

Were any players on during that time?

[sorvani]

Not when i came down no. It is mainly just used by me to test things.
I had toons in and out yesterday most of the day and it was running.
Went to bed early and when i came down to my office this morning UCS was crashed.

UCS only crashes on a new log in that I've noticed, but then the times that I've seen it actually crash were when I logged in a toon after being offline for hours and no one else had logged in during the interim.

I restarted the UCS right when I seen it crashed this morning and I have had a couple toons in and out of my server this afternoon and it hasn't crashed again yet.

[Zothen]

My UCS is running in visual studio debug mode all of the time. Got a database error actually today, but no crash.

[Zothen]

I analyzed the UCS code a bit further and I am now 99.99% sure that my fix is the way it is supposed to be.

When the function dbcore::RunQuery() results in a connection/database error, it is calling itself again for exactly 1 retry, but on that 2nd call it checks for the status. If pStatus = error then dbcore::Open() is called.

Open() calls mysql_close() on any error status and is supposed to reconnect in the same call, but will definitetely fail without mysql_init(). So placing a FALSE after the "close" or "init" would prevent Open() to reconnect.

All that is missing now is a confirmation message, that the recovery of the encountered error was successful.

bool DBcore::RunQuery(const char* query, int32 querylen, char* errbuf, MYSQL_RES** result, int32* affected_rows, int32* last_insert_id, int32* errnum, bool retry) {
	_CP(DBcore_RunQuery);
	if (errnum)
		*errnum = 0;
	if (errbuf)
		errbuf[0] = 0;
	bool ret = false;
	LockMutex lock(&MDatabase);
	if (pStatus != Connected)
		Open();
#if DEBUG_MYSQL_QUERIES >= 1
	char tmp[120];
	strn0cpy(tmp, query, sizeof(tmp));
	cout << "QUERY: " << tmp << endl;
#endif
	if (mysql_real_query(&mysql, query, querylen)) {
		if (mysql_errno(&mysql) == CR_SERVER_GONE_ERROR)
			pStatus = Error;
		if (mysql_errno(&mysql) == CR_SERVER_LOST || mysql_errno(&mysql) == CR_SERVER_GONE_ERROR) {
			if (retry) {
				cout << "Database Error: Lost connection, attempting to recover...." << endl;
				ret = RunQuery(query, querylen, errbuf, result, affected_rows, last_insert_id, errnum, false);
				if ( !ret )
					cout << "Reconnection to database successful." << endl;
			}
			else {

...
...
...

Unfortunetely, that doesn't explain the latest crash of yours, sorvani. This may be an other error I have not encountered yet.

[sorvani]

Looked at my server this morning and the chat service was still running. Looked at the character_ table and it look like only a single random toon signed on after the last time I was on.
I started my client and connected. As soon I clicked enter world the UCS crashed. With the standard WIndows XP please report dialog box.
In the command window:

Database Error: Lost connection, attempting to recover....

Character	TimeLastOn
TestBeasti	1304535868 <- working
GMSorvani	1304535876 <- working
XXXXXX		1304540215 <- working
TestBeasti	1304602955 <- crashed at enter world

[Zothen]

Aye, thats the error I encountered last time, but for me it doesnt crash, it actually recovers now.

[sorvani]

Looked at my build again and found an error on my side. When I added the mysql_init I had a syntax error. Rebuilt and restarted.