Attempt to read or write protected memory when value to pointer

Question

I have this code:

typedef struct {
    string fName;
    string str; 

}t;

//-------Other functions------//
void BeginTh()
{
    string arg = "yes";
    t *arglist;
    arglist = (t*)malloc(sizeof(t));
    arglist->fName = "comBomber";
    arglist->str = arg;
    _beginthread(startOver, 0, (void*)arglist);
    free(arglist);
}

And at 'arglist->fName = "comBomber";' i get this error:

An unhandled exception of type 'System.AccessViolationException' occurred in <appname>

Additional information: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

Anyone can help me ? How solve this problem ?

Thanks.

No offense meant, but this code is wrong and dangerous on so many levels, I wouldn't know where to start, You _really_, urgently, need to read [a good C++ book](http://stackoverflow.com/q/388242/140719). — sbi, Nov 24 '12 at 11:36
@sehe: That depends.You cannot say that through the code that is shown in the Q. — Alok Save, Nov 24 '12 at 11:45
Which language is this intended to be? It looks like C, is tagged as C++, and you seem to be getting a C++/CLI compiler error. So it seems like it would be in order to ask which language you *want* to write :) — jalf, Nov 24 '12 at 12:03
@Als are you getting at preventing copy (see move semantics) or shared ownership (see shared_ptr) — sehe, Nov 24 '12 at 15:43

score 6 · Answer 1 · answered Nov 24 '12 at 12:12

6

I suggest modern C++ style:

#include <future>
#include <string>

struct arg_t {
    std::string fName;
    std::string str; 
};

int startOver(int i, arg_t args)
{
    return args.fName.size() + args.str.size() + 42;
}

int main()
{
    const std::string arg = "yes";
    arg_t args = { "comBomber", arg };
    auto worker = std::async(startOver, 0, args);
    return worker.get();
}

See it on http://ideone.com/zrXcwH (it doesn't run because ideone doesn't support the pthread library). I tested this with MS Visual C++.

If arg_t would be very expensive to copy, you can simply move it to the other thread:

auto worker = std::async(startOver, 0, std::move(args));

answered Nov 24 '12 at 12:12

sehe

374,641
47
450
633

+1, but minor nit-pick: I think you need to pass `std::launch::async` to ensure the function is executed in another thread, otherwise it might just to lazy execution. – juanchopanza Nov 24 '12 at 12:31
@juanchopanza YOu're right, i.e. if the goal is too mimic the original semantics. (On the other hand, this subtle difference might actually be _another big benefit_ of using the 'highlevel' portable interfaces) – sehe Nov 24 '12 at 15:42

juanchopanza · Accepted Answer · 2012-11-24T12:28:30.533

One problem is that your t instance is not properly initialized. You can fix that by it by using new instead of malloc Your struct holds a string, whose constructor needs to be called. Calling new ensures that the t object gets constructed properly.

 t* arglist = new t;

then "free" the memory by calling delete:

delete arglist;

This points to the second problem, which is that your t instance must be guaranteed to be alive during the whole execution of the thread. You should not de-allocate its memory until the thread is finished. This is a C++ example where the t object is guaranteed to outlive the thread:

#include <thread>

int main()
{
  t arglist = whatever;
  std::thread t(startover, &whatever); // launches thread which runs startover(&arglist)

  // do other stuff

  t.join(); // wait for thread execution to finish

}

In general, Instead of using raw pointers to dynamically allocated objects, you should use a smart pointer.

As an aside, the typedef syntax for declaring a struct looks pretty strange in C++. Normally, you would do this:

struct t {
    string fName;
    string str; 
};

How would that solve his problems? The object will still be gone by the time the new thread is laying its hands on it. — sbi, Nov 24 '12 at 11:38
@sbi thanks, I had completely missed that. Not being familiar with `_beginthread`, all I can say is "make sure your object is not deallocated until the thread is finished". — juanchopanza, Nov 24 '12 at 11:45
What? Why would it be unable to cope with user-defined types? — jalf, Nov 24 '12 at 12:04

billz · Answer 3 · 2012-12-03T02:45:21.350

3

malloc will only allocate memory for object but will not call its constructor

you need change to new

t *arglist = new t;

Also, do not release arglist memory block before the startOver thread gets its content. you can release it inside the thread or somewhere else.

void startOver(void* param)
{
  Param* param_ = (Param*)param;  // get hold of param pointer
  while(true){
    // do something
   }
  delete param_;  // release param when thread finishes
}

void BeginTh()
{
    Param *param = new Param();
    param->fName = "abcd";
    param->str = "yes";
    _beginthread(startOver, 0, (void*)param);
 }

edited Dec 03 '12 at 02:45

answered Nov 24 '12 at 11:34

billz

44,644
9
83
100

I am tempted to downvote this just because it shows `free()` as a possible match to `new`. Even though it's commented out, it _could_ give someone wrong ideas. – sbi Dec 01 '12 at 20:35

Attempt to read or write protected memory when value to pointer

3 Answers3