Prev: Force loading of DLL
Next: Default intialization without default constructor
From: Rayne on 2 Mar 2010 22:12
Hi all,

I'm having some problems with my program in VS .NET 2003.

I initially wrote a module that uses the pthread library to create a
number of threads to process something. This runs correctly in VS .NET
2003. Then this module was used by someone else and integrated into
another larger program. I'm not sure about the details, but the
program creates a GUI that allows the user to select an option to run
my module.

When the thread is created, a value is passed in as the thread id. The
problem with my module in the GUI is that the value of the thread id
is 0 for all the threads, while the thread id is correct in the module
without GUI.

Here's how the thread is created in the module:

int64_t *tid[1000];
int64_t i = 0, rc;

for (i = 0 ; i < NUM_THREADS ; i++)
{
tid[i] = (int64_t *) malloc(sizeof(int64_t));
*tid[i] = i;
rc = pthread_create(&pthread, &attr, function, (void *)tid[i]);
Sleep(1);
if(rc)
{
free(tid[i]);
exit(1);
}
free(tid[i]);
}

I checked the project properties of both, and the only differences
between the 2 projects are listed below:

GUI - use managed extensions | my module (w/o GUI) - does not use
managed extensions
In C/C++ preprocessor:
GUI - WIN32;_DEBUG;_CONSOLE;WINDOWS | my module (w/o GUI) - none
In C/C++ Additional Options:
GUI - /CLR | my module (w/o GUI) - no /CLR (error with /CLR: fatal
error LNK1000: Internal error during BuildImage)

The code is the same, so I don't get why the output is wrong for the
GUI, unless the use of managed extensions/clr somehow makes a
difference? (I'm not really sure what those are either.)

Please advise.

Thank you.

Regards,
Rayne
From: Tamas Demjen on 3 Mar 2010 17:25
Rayne wrote:

> rc = pthread_create(&pthread, &attr, function, (void *)tid[i]);
> Sleep(1);
> if(rc)
> {
> free(tid[i]);
> exit(1);
> }
> free(tid[i]);

You are passing a pointer to your new thread, but 1 millisecond later
that pointer gets deleted. Chances are by the time the thread is awake
the pointer is already invalid (addressing random garbage). If this
code works at all, it's due to pure luck (and the Sleep function). There
is a high risk of failure there, and sooner or later you are going to
get into random memory corruption errors, which is going to be extremely
difficult to debug.

One solution would be to free the pointer from the thread instead:
void ThreadFunction(void* param)
{
int* pid = (int*)param;
int id = *pid;
free(pid);

Of course the "free(tid[i]);" line must go away when pthread_create
succeeds.

If you feel really lazy, that void* pointer can be used to hold the
integer value directly:

rc = pthread_create(&pthread, &attr, function, (void *)i);

This is not a very elegant solution, though. It is confusing and very
easy to overlook. It's not what a programmer would expect from a
pointer. Of course the thread will have to treat this pointer as an
integer as well:

void ThreadFunction(void* param)
{
int id = (int)param; // careful, void* was reinterpreted as int

Such a thing raises an eyebrow. Another disadvantage is that it
won't let you add a second argument if you need one in the future.
I'd use a pointer to a structure instead.

Tom
From: Rayne on 3 Mar 2010 21:02
On Mar 4, 6:25 am, Tamas Demjen <tdem...(a)yahoo.com> wrote:
> Rayne wrote:
> > rc = pthread_create(&pthread, &attr, function, (void *)tid[i]);
> > Sleep(1);
> > if(rc)
> > {
> > free(tid[i]);
> > exit(1);
> > }
> > free(tid[i]);
>
> You are passing a pointer to your new thread, but 1 millisecond later
> that pointer gets deleted. Chances are by the time the thread is awake
> the pointer is already invalid (addressing random garbage). If this
> code works at all, it's due to pure luck (and the Sleep function). There
> is a high risk of failure there, and sooner or later you are going to
> get into random memory corruption errors, which is going to be extremely
> difficult to debug.
>
> One solution would be to free the pointer from the thread instead:
> void ThreadFunction(void* param)
> {
> int* pid = (int*)param;
> int id = *pid;
> free(pid);
>
> Of course the "free(tid[i]);" line must go away when pthread_create
> succeeds.
>
> If you feel really lazy, that void* pointer can be used to hold the
> integer value directly:
>
> rc = pthread_create(&pthread, &attr, function, (void *)i);
>
> This is not a very elegant solution, though. It is confusing and very
> easy to overlook. It's not what a programmer would expect from a
> pointer. Of course the thread will have to treat this pointer as an
> integer as well:
>
> void ThreadFunction(void* param)
> {
> int id = (int)param; // careful, void* was reinterpreted as int
>
> Such a thing raises an eyebrow. Another disadvantage is that it
> won't let you add a second argument if you need one in the future.
> I'd use a pointer to a structure instead.
>
> Tom

Thank you for your comprehensive explanation! This was the problem and
freeing the tid[i] value from the worker thread solved it.
 | 
Pages: 1
Prev: Force loading of DLL
Next: Default intialization without default constructor