Crash in world server when instance is created about server HOT 14 CLOSED

I do not get a crash. Server 2012 using a 64 bit build, MySQL 5.1, Perl 5.14

from server.

MySQL version? MariaDB maybe?

from server.

Note, that was my push to the code and tested on my windows server before being pushed live.

from server.

Irrelevant, really, the original code is broken. It frees the result, then uses it after freeing it. If it works, consider yourselves "lucky", that on your systems the memory the result points to didn't get immediately invalidated after being freed.

from server.

If the way it currently is is bad, then pretty much every SQL statement in the code is bad :P

from server.

It's usually doing the frees after using the result in the rest of the code. In this instance, it looks like someone tried to catch a memory leak but put the free in a bit too early.

In any case it's no big deal. I've encountered this with VS2010 and MySQL 5 forced in MYSQL40 mode. And yes, there are many more instances where code is not proper, but may work with certain compilers or compiler settings.

from server.

Prior to deleting the result, it is dumped into another variable. That variable is then what is referenced. Or are you implying the deleting the result then reusing the same variable in the next query statement is the issue?

from server.

The typedef for MYSQL_ROW is a pointer. Apparently my version of mysql_fetch_row returns a pointer to the value inside the result (and not a copy thereof), which is subsequently invalidated when the result is cleared. I've traced this code, and row loses its value the moment the result is freed.

from server.

FYI I am linking vs the "official" windows x86 dependencies.

from server.

I'd like to add that if row became a copy of something inside the result, you'd probably have to free it at the end or have a memory leak. I don't see that row is freed anywhere, so I must assume that it works as intended for me.

from server.

My test server is intentionally 100% default 64-bit dependencies and versions linked in the public tutorials. Compiled 64-bit, with Lua enabled, for VS 2012.

Try actually posting some details so someone can attempt to replicate your issue. As mackal stated, this exact use is all over the eqemu code and causes no one else any issues. This implies that you are doing something no one else is doing.

from server.

I've given you all the details available. You're presuming that I am doing "something", when in fact all I am doing is compiling the source off of GIT without modifications.

Your statement that this particular query code "is all over the place" is unqualified and from a cursory look at the code, incorrect. Let's see some random samples:

        if (mysql_num_rows(result) > 0)
        {
            row = mysql_fetch_row(result);
            while(row != nullptr)
            {
                id = atoi(row[0]);
                DeleteInstance(id);
                row = mysql_fetch_row(result);
            }
        }
        mysql_free_result(result);

row is used, and then the result is freed.

    if (RunQuery(query, MakeAnyLenString(&query, "SELECT account_id FROM character_ WHERE id=%i", char_id), errbuf, &result)) {
        if (mysql_num_rows(result) == 1) {
            row = mysql_fetch_row(result);
            ret = atoi(row[0]); // copy to temp var because gotta free the result before exitting this function
        }
        mysql_free_result(result);
    }

row is used, then the result is freed.

Now let's look at the current code in GetUnusedInstanceID:

    if (mysql_num_rows(result) != 0) {
row = mysql_fetch_row(result);
mysql_free_result(result);
if(atoi(row[0]) <= max) {
count = atoi(row[0]);
} else {

The row pointer is initialized by the calll to mysql_fetch_row. What this does is it returns an array of character arrays to the columns of the result. Next statement, the result is freed, which invalidates the result and causes the pointers in the row pointer to point to memory that has effectively been deallocated. Finally, row[0] is referenced in the if, but the state of row[0] is now indeterminate because the underlying memory has been deallocated. If the code works, then only because the deallocated memory is not overwritten by the time it is used. Nevertheless, the code is NOT doing it right. And if you bother to look at the other queries in database.cpp, you will realize that they don't free the result set before accessing the values in the row array.

I hope this is detailed enough.

from server.

Detailed meant to detail your install process because right or wrong it is not crashing for others. I have understood your crash issue since the first post.

Your solution to use a second pointer and move the free is also not accurately looking at how the information was used. the row you are so worried about is used exactly once. so moving the free_result to directly after that will eliminate that concern and should fix your crash.
Then free it in the else prior to re-using it instead of using a second pointer. all other instance of the free_result are not going to affect anything.
Unrelated i need to rewrite the entire else section to be cleaner with a better SQL statement, but that is the original code that has been in use for years so I put off changing that more than to place it in the else from the first query.

            row = mysql_fetch_row(result);
            if(atoi(row[0]) <= max) {
                count = atoi(row[0]);
                mysql_free_result(result);
            } else {
                // free result from prior query in order to use variable again.
                mysql_free_result(result);
                if (RunQuery(query, MakeAnyLenString(&query, "SELECT id FROM instance_list where id > %u ORDER BY id", count), errbuf, &result)) {

from server.

While I understand the need to try and replicate the behaviour, it doesn't really matter in this case, where we can demonstrate that the code is obviously - logically - broken.

Your own version of the fix to the issue looks fine to me - different coding style, same result.

from server.