Global array variable loses values in thread

Question

I am developing a little application that first adds some values to an array of pointers defined as a global variable:

#define MAX_LOCATIONS 32

LocationDTO* locations[MAX_LOCATIONS];
int nLocations = 0;

I use the following method to add the LocationDTO object references to the array:

bool addLocation(Location *location, string name){
    LocationDTO l(name, location);
    if(nLocations < MAX_LOCATIONS){
        locations[nLocations] = &l;
        nLocations++;
        return true;
    }else{
        return false;
    }
}

And I call this function from the main method this way:

Location l1(1,2);
addLocation(&l1, "one");

Location l2(2,3);
addLocation(&l2, "two");

Location l3(4,5);
addLocation(&l3, "three");

After all values are added I start a thread that will process this array. Thread definition:

void* server_thread(void* args){
    // Some code

    int i;
    for(i=0; i < nLocations; i++){
        LocationDTO* l = locations[i];

        // More Code
    }
}

The problem is that, at this point in the thread, the objects contained inside locations don't have anymore the values I assigned to them.

Does this problem happen because I am creating the objects inside addLocation and then saving the reference in the array?

l is a local variable. It goes away when the function ends. If you want to save the value between calls use the new operator to allocate the memory on the heap. — caveman
– caveman, Commented Feb 19, 2015 at 15:36

Mikhail Khorolets · Accepted Answer · 2015-02-19 15:43:41Z

2

Yes you are right. LocationDTO l variable will be destroyed when you leave addLocation method.

i see few possible solutions for this problem: first solution:

std::vector<LocationDTO> locations;
const size_t maxSize = 32;
bool addLocation(Location *location, string name)
{
    if( locations.size() < maxSize )
    {
        locations.emplace_back( name, location );
        return true;
    }
    return false;
}

second solution:

bool addLocation(Location *location, string name){
    LocationDTO* l = new LocationDTO(name, location);
    if(nLocations < MAX_LOCATIONS){
        locations[nLocations] = l;
        nLocations++;
        return true;
    }else{
        return false;
    }
}

if you decide to use second solution don't forget to delete all allocated LocationDTO objects

answered Feb 19, 2015 at 15:43

Mikhail Khorolets

961 silver badge4 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

YoungJohn · Accepted Answer · 2015-02-19 15:38:38Z

0

In addLocation you are creating a local variable l, and storing the address of l in your global array. At the end of addLocation any local variables in addLocation become invalid as they are destroyed. The memory at the address of what once was l may, and will, be used for other data. You must use some form of dynamic memory allocation to allow the memory at that location to persist beyond the scope of the addLocation function.

answered Feb 19, 2015 at 15:38

YoungJohn

97613 silver badges19 bronze badges

Comments

ezaquarii · Accepted Answer · 2015-02-19 15:39:10Z

0

It seems that you create those objects on stack and pass the pointers to addLocation() method. Those objects are destroyed when you leave the scope (exit function). Are you sure those objects exist when you start the thread?

Try this:

addLocation(new Location(1,2), "one");
addLocation(new Location(2,3), "two");
addLocation(new Location(4,5), "three");

Of course you must destroy those objects later.

answered Feb 19, 2015 at 15:39

ezaquarii

1,91613 silver badges15 bronze badges

Comments

eerorika · Accepted Answer · 2015-02-19 15:50:03Z

The problem is that, at this point in the thread, the objects contained inside locations don't have anymore the values I assigned to them.

Your locations array don't contain LocationDTO objects. It contains pointers to objects.

Does this problem happen because I am creating the objects inside addLocation and then saving the reference in the array?

Yes, almost. More exactly, your bug is that you point the pointer in locations to an automatic variable. Automatic variables are destroyed when they go out of scope (at the end of addLocation in this case) and after that the pointers are no longer pointing to a valid object. References are not the same thing as pointers, but this would also apply to them.

The simplest way to avoid bugs with memory handling is to let the standard library take care of it. I recommend you use std::vector<LocationDTO> locations;

Collectives™ on Stack Overflow

Global array variable loses values in thread

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related