Hello all,
I noticed a bug testing my gphoto code. It was written for 40-3 but does no longer work in 42.4.
The problem is passing the A_GIMME arguments from a wrapper function to a function that runs in a separate thread.
This is done by populating a structure mimicking the A_GIMME args:
typedef struct gphoto_gimme_struct { gphoto_struct *gphoto; t_symbol *s; int argc; t_atom *argv; } gphoto_gimme_struct;
This struct is populated like so in the function that spawns the thread:
static void wrapCaptureImage(gphoto_struct *gphoto, t_symbol *s, int argc, t_atom *argv) { int ret; pthread_t thread1; t_symbol *filename;
if (!gphoto->busy) {
// instance of structure gphoto_gimme_struct *threadArgs = (gphoto_gimme_struct *)malloc(sizeof(gphoto_gimme_struct));
// packaging arguments into structure threadArgs->gphoto = gphoto; threadArgs->s = s; threadArgs->argc = argc; threadArgs->argv = argv;
// We're busy gphoto->busy = 1;
// Create thread ret = pthread_create( &thread1, &gphoto->threadAttr, captureImage, threadArgs); } else { error("gphoto: ERROR: Already executing a command, try again later."); } }
When I print out the value of argv in the above function all is well.
In 42.4 the function that runs in the thread sees argv differently than the spawning function.
For example:
post("out: argv: %s", atom_getsymbol(argv)->s_name);
In the spawning function properly prints out the argument supplied in PD (for example input.jpg)
In the function that runs in the thread the argument is printed as follows:
post("in: argv: %s", atom_getsymbol(((gphoto_gimme_struct *)threadArgs)->argv)->s_name);
"Float" is printed to the console, not "input.jpg" as supplied in PD.
In PD <.0.40.3 the proper argument is printed from both the thread and in the spawning function.
I'm at a loss here.
How does the symbol "input.jpg" end up turning into "float" in 0.42.4?
Any ideas for solutions?
All the code is in svn/externals/bbogart/gphoto
Thanks, B. Bogart
Hi,
B. Bogart wrote:
The problem is passing the A_GIMME arguments from a wrapper function to a function that runs in a separate thread.
[snip]
This is the thread safety problem right here:
threadArgs->argv = argv;
You need to *copy* *all* the data needed by the child thread that might be modified in the main thread later.
[snip]
In PD <.0.40.3 the proper argument is printed from both the thread and in the spawning function.
You've just been lucky!
How does the symbol "input.jpg" end up turning into "float" in 0.42.4?
It could turn into anything - when the inlet method exits pd can do what it likes with the argv memory...
Any ideas for solutions?
Copy the argv array into new memory.
Claude
Thanks Claude,
I'm not done much (almost none) mem management in C.
As far as I understand it my struct is being allocated a new space in memory with:
gphoto_gimme_struct *threadArgs = (gphoto_gimme_struct *)malloc(sizeof(gphoto_gimme_struct));
So rather than copying the references to gphoto, s, argc and argv I should copy the data at those locations into the newly allocated struct. I tried as follows:
memcpy(&threadArgs->gphoto, &gphoto, sizeof(gphoto)); memcpy(&threadArgs->s, &s, sizeof(s)); memcpy(&threadArgs->argc, &argc, sizeof(argc)); memcpy(&threadArgs->argv, &argv, sizeof(argv));
I think I got the syntax right, as I can access the values.
Problem is the same issue persists. The argument still ends up being "float" when read in the thread.
So what have I misunderstood?
Thanks for your help, B. Bogart
Claude Heiland-Allen wrote:
Hi,
B. Bogart wrote:
The problem is passing the A_GIMME arguments from a wrapper function to a function that runs in a separate thread.
[snip]
This is the thread safety problem right here:
threadArgs->argv = argv;You need to *copy* *all* the data needed by the child thread that might be modified in the main thread later.
[snip]
In PD <.0.40.3 the proper argument is printed from both the thread and in the spawning function.
You've just been lucky!
How does the symbol "input.jpg" end up turning into "float" in 0.42.4?
It could turn into anything - when the inlet method exits pd can do what it likes with the argv memory...
Any ideas for solutions?
Copy the argv array into new memory.
Claude
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Thanks Martin,
I can't get the syntax right, care to give me a hint?
B. Bogart
martin.peach@sympatico.ca wrote:
So what have I misunderstood?
argv is a pointer, it's size is 4. You're still not copying whatever argv is pointing to.
Martin
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Maybe easiest and safest is to make sure sizeof(gphoto_gimme_struct) is big enough to hold some maximum number of args by declaring it a fixed size:
#define MAXARGS 339966 t_atom argv[MAXARGS];
instead of
t_atom *argv;
and then copy up to MAXARGS atoms in a loop:
for (i = 0; (i < argc)&&(i < MAXARGS); ++i) threadargs->argv[i] = argv[i];
instead of
threadArgs->argv = argv;
...or you could leave the struct alone and separately allocate the precise amount of memory for argc atoms, as
threadArgs->argv = malloc(sizeof(*argv)*argc));
and then copy them over.
Martin
B. Bogart wrote:
Thanks Martin,
I can't get the syntax right, care to give me a hint?
B. Bogart
martin.peach@sympatico.ca wrote:
So what have I misunderstood?
argv is a pointer, it's size is 4. You're still not copying whatever argv is pointing to.
Martin
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Hi Martin,
So that worked for getting the proper value, but the method I was using to get the second argument (argv+1) no longer works. Presumably because the "next" location is no longer the same.
So should I chuck this idea of using a single struct that mirrors the A_GIMME arguments and multiple structs specific to each function?
One reason for making the threadargs struct was the possibility of using a single function to wrap and execute all threads, just passing the args over for the thread function to make sense of them (though I could not figure out how to use a single wrapper).
Or is there a way to get at the second argument in threadargs->argv I'm not seeing?
I've not committed the changed code using malloc, but you can get the gist from the code currently in:
svn:pure-data/trunk/externals/bbogart/gphoto
Thanks Martin, B. Bogart
PS: I'll be exhibiting DM in Montreal for Elektra this summer!
Martin Peach wrote:
Maybe easiest and safest is to make sure sizeof(gphoto_gimme_struct) is big enough to hold some maximum number of args by declaring it a fixed size:
#define MAXARGS 339966 t_atom argv[MAXARGS];
instead of
t_atom *argv;and then copy up to MAXARGS atoms in a loop:
for (i = 0; (i < argc)&&(i < MAXARGS); ++i) threadargs->argv[i] = argv[i];instead of
threadArgs->argv = argv;...or you could leave the struct alone and separately allocate the precise amount of memory for argc atoms, as
threadArgs->argv = malloc(sizeof(*argv)*argc));and then copy them over.
Martin
B. Bogart wrote:
Thanks Martin,
I can't get the syntax right, care to give me a hint?
B. Bogart
martin.peach@sympatico.ca wrote:
So what have I misunderstood?
argv is a pointer, it's size is 4. You're still not copying whatever argv is pointing to.
Martin
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
ben wrote:
... So that worked for getting the proper value, but the method I was using to get the second argument (argv+1) no longer works. Presumably because the "next" location is no longer the same. ...
Do you mean after you copied the data? Did you try either
threadargs->argv[1]
or
*(threadargs->argv+1)
? They should both give the value of the second argument.
Martin
Yes after copying.
gcc does not like either, both result in:
error: incompatible type for argument 1 of 'atom_getint'
This is the old way:
intValue = atom_getint( ((gphoto_gimme_struct *)threadArgs)->argv+1 ); post("in: config value: %d", intValue);
and I tried both these:
intValue = atom_getint( *(((gphoto_gimme_struct *)threadArgs)->argv+1) );
intValue = atom_getint( ((gphoto_gimme_struct *)threadArgs)->argv[1] );
Same thing happens when I try to access the second argument from the wrapper function, from the threadargs struct.
.b.
martin.peach@sympatico.ca wrote:
ben wrote:
... So that worked for getting the proper value, but the method I was using to get the second argument (argv+1) no longer works. Presumably because the "next" location is no longer the same. ...
Do you mean after you copied the data? Did you try either
threadargs->argv[1]
or
*(threadargs->argv+1)
? They should both give the value of the second argument.
Martin
That's because atom_getint() wants a pointer to an atom, not the atom itself. Try:
intValue = atom_getint(((gphoto_gimme_struct *)threadArgs)->argv+1);
or:
intValue = atom_getint(&((gphoto_gimme_struct *)threadArgs)->argv[1]);
Martin
ben wrote:
Yes after copying.
gcc does not like either, both result in:
error: incompatible type for argument 1 of 'atom_getint'
This is the old way:
intValue = atom_getint( ((gphoto_gimme_struct *)threadArgs)->argv+1 ); post("in: config value: %d", intValue);
and I tried both these:
intValue = atom_getint( *(((gphoto_gimme_struct *)threadArgs)->argv+1) );
intValue = atom_getint( ((gphoto_gimme_struct *)threadArgs)->argv[1] );
Same thing happens when I try to access the second argument from the wrapper function, from the threadargs struct.
.b.
martin.peach@sympatico.ca wrote:
ben wrote:
... So that worked for getting the proper value, but the method I was using to get the second argument (argv+1) no longer works. Presumably because the "next" location is no longer the same. ...
Do you mean after you copied the data? Did you try either
threadargs->argv[1]
or
*(threadargs->argv+1)
? They should both give the value of the second argument.
Martin
Pd-dev mailing list Pd-dev@iem.at http://lists.puredata.info/listinfo/pd-dev
-
B. Bogart -
Claude Heiland-Allen -
Martin Peach -
martin.peach＠sympatico.ca