New GPS functionality for the SX230 - page 2 - General Discussion and Assistance - CHDK Forum

New GPS functionality for the SX230

  • 34 Replies
  • 14338 Views
Advertisements
I'm going to try and capture the essence of a long offline conversation philmoz and I had about crashing while working with loadable modules (as that was what I was attempting to do with the GPS code).  Text in quotes are from things philmoz pointed out.

  • "It could also be related to two tasks trying to load/unload modules at the same time - the module load code is definitely not thread safe."(in my new code both spytask and the keyboard task try to load the gps module at different times)
  • "It may be that memory allocation / freeing are not thread safe, and calling GetMemInfo while another task is doing memory stuff is not safe. The module loader allocates an 'uncached' buffer to load the file, and frees it afterwards - this should probably be changed to keep the buffer for the next module load." (in response to the crashes reported in my previous post in this thread)
  • "Given the complexity of the GPS code, I'm not sure how safe this will be as a module - never really anticipated a module starting it's own tasks."(the dng flt module does but seems to be the only one)
  • "Module loading (and probably unloading) is not safe. If a task tries to load a module while another task is already in the middle of a load then it will cause chaos. This should be pretty unusual or we would have had reports already; but there are some changes I can make to minimise the risk" (my torture test code was purposely loading/unloading 20 times a second so I probably tripped over this)
  • "Multiple tasks writing to the screen is probably also not safe. Some of the drawing functions use global variables (fill routines). Probably won't crash; but you may get odd results. The drawing code can be in a module; but it should be called from the draw routine in the gui_handler." (I have not looked to see if this is always the case but its certainly a problem for the existing gps code..)

So at this point, I'm going to abandon myconversion of the existing GPS code to module format. For cameras without GPS, there was no memory penalty from the GPS code anyway.

This still leaves dealing with the lack of a mutex mechanism for the information shared between the six GPS tasks and a look at comment five from above - writing to the screen from multiple tasks.
Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline reyalp

  • ******
  • 14080
"It may be that memory allocation / freeing are not thread safe, and calling GetMemInfo while another task is doing memory stuff is not safe. The module loader allocates an 'uncached' buffer to load the file, and frees it afterwards - this should probably be changed to keep the buffer for the next module load." (in response to the crashes reported in my previous post in this thread)
I believe firmware malloc/free must be thread safe, since firmware functions use them all over the place. suba probably isn't, and probably should be. We always use system heap for uncached memory.

Multiple tasks using a global for the buffer could still lead to problems, of course.
Quote
"Given the complexity of the GPS code, I'm not sure how safe this will be as a module - never really anticipated a module starting it's own tasks."(the dng flt module does but seems to be the only one)
I don't think there should be any inherent problem with modules starting their own tasks, as long as they ensure the module remains loaded while the tasks are running.

FWIW, in DNG the reverser task is created for each DNG save and exits when it is complete. The main task waits for this to happen. There might be a tiny window between rb_state.stage = RB_STAGE_DONE and ExitTask, but for DNG file saving, the module is only loaded/unloaded when the option changes. remotecap can also cause the module to be loaded/unloaded, but it never starts the reverser.
Don't forget what the H stands for.

I don't think there should be any inherent problem with modules starting their own tasks, as long as they ensure the module remains loaded while the tasks are running.
Struggling with how to do that so it always works.   

In most of the current module implementations, there is a "running" variable (semaphore/flag) used to indicate whether the module is safe to unload or not.  If running is cleared just before a call to ExitTask(); there is a small window between the two instructions where the module could get unloaded prior to the task exiting.   

I think this sort of thing is usually handled at the OS level in most systems.  CHDK hacks a loadable module format on top of the OS that the tasking mechanism in unaware of. 

Or did I miss something simple here? (again).

Edit :  is there a way for the task to unload itself rather than wait for the code in spytask to do it via the original routine called when the module was loaded ?
« Last Edit: 29 / November / 2014, 18:33:35 by waterwingz »
Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline philmoz

  • *****
  • 3450
    • Photos
I don't think there should be any inherent problem with modules starting their own tasks, as long as they ensure the module remains loaded while the tasks are running.
Struggling with how to do that so it always works.   

In most of the current module implementations, there is a "running" variable (semaphore/flag) used to indicate whether the module is safe to unload or not.  If running is cleared just before a call to ExitTask(); there is a small window between the two instructions where the module could get unloaded prior to the task exiting.   

I think this sort of thing is usually handled at the OS level in most systems.  CHDK hacks a loadable module format on top of the OS that the tasking mechanism in unaware of. 

Or did I miss something simple here? (again).

That pretty much sums it up - welcome to the house of cards :)

The DNG module is reasonably safe since it won't unload the module while DNG saving is enabled in the CHDK menu (to prevent the module unloading after every picture). Users can't disable DNG while a picture is being saved.

Phil.
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)


That pretty much sums it up - welcome to the house of cards :)
This takes me way back in time too far to reveal. Old problem - same issues.

Edited my older post while you were posting this.   Seems somehow appropriate give the subject matter.  My question was :
 
Is there a way for the task to unload itself rather than wait for the code in spytask to do it via the original routine called when the module was loaded ?

Update :  thinking about the question - it's the same problem. 
« Last Edit: 29 / November / 2014, 18:58:51 by waterwingz »
Ported :   A1200    SD940   G10    Powershot N    G16

*

Offline philmoz

  • *****
  • 3450
    • Photos
Is there a way for the task to unload itself rather than wait for the code in spytask to do it via the original routine called when the module was loaded ?

Not in the current implementation. Can't see how it would help in any case.
(I assume you meant module, not task.)

Phil.
CHDK ports:
  sx30is (1.00c, 1.00h, 1.00l, 1.00n & 1.00p)
  g12 (1.00c, 1.00e, 1.00f & 1.00g)
  sx130is (1.01d & 1.01f)
  ixus310hs (1.00a & 1.01a)
  sx40hs (1.00d, 1.00g & 1.00i)
  g1x (1.00e, 1.00f & 1.00g)
  g5x (1.00c, 1.01a, 1.01b)
  g7x2 (1.01a, 1.01b, 1.10b)

*

Offline reyalp

  • ******
  • 14080
In most of the current module implementations, there is a "running" variable (semaphore/flag) used to indicate whether the module is safe to unload or not.  If running is cleared just before a call to ExitTask(); there is a small window between the two instructions where the module could get unloaded prior to the task exiting.   
One way to deal with this would be to force some delay after the module declared itself done. E.g can_unload could do something like
Code: [Select]
static int unload_time=0;
int _module_can_unload()
{
    if (running == 1) {
      unload_time = 0;
      return 0;
    }
// running == 0
    if (!unload_time) {
// previous state was running, don't allow unload for 1 sec
       unload_time = get_tick_count() + 1000;
       return 0;
    }
// has unload time passed
    return unload_time < get_tick_count();
}
You'd have to think about what would happen if the module was needed again before the unload delay expired. Depending on how 'running' is set, there could be race conditions there.

You could also do something similar in module_tick_unloader.

I expect there are also OS functions that could help (perhaps something like atexit()? or something that waits on some kind of task handle) but I don't know what they are if they exist.
Don't forget what the H stands for.

One way to deal with this would be to force some delay after the module declared itself done.
Probably more than good enough. But the "how long a delay" conversation and the "it wont' always work" have been played out thousands of times I'm sure.  Unless you can disable interrupts, all of this is just hoping.
Ported :   A1200    SD940   G10    Powershot N    G16


*

Offline reyalp

  • ******
  • 14080
Probably more than good enough. But the "how long a delay" conversation and the "it wont' always work" have been played out thousands of times I'm sure.  Unless you can disable interrupts, all of this is just hoping.
*shrug*
There's tons of stuff that's technically unsafe in CHDK, some of it discussed just a few posts back. I would be shocked if a few hundred ms wasn't enough to completely resolve the issue for all practical purposes. If this scenario is the cause of the problem at all...

edit:
Another approach would be to make sure the code that sets the final "ok to unload" flag and calls ExitTask is in the core rather than the module, something like
Code: [Select]
module_exit_task(int *flag) {
 *flag=1;
 ExitTask();
}
« Last Edit: 29 / November / 2014, 23:34:38 by reyalp »
Don't forget what the H stands for.

Another approach would be to make sure the code that sets the final "ok to unload" flag and calls ExitTask is in the core rather than the module, something like
Code: [Select]
module_exit_task(int *flag) {
 *flag=1;
 ExitTask();
}
Wow .. so simple & elegant it's completely brilliant.  As the best solutions always are.

Thanks.
Ported :   A1200    SD940   G10    Powershot N    G16

 

Related Topics