a09b0e2357f2 Remove reversal of children in Fl_Group::clear() (Albrecht Schlosser)
Causes my program to attempt to double-free a widget on shutdown.
My program subclasses Fl_Group. My specialized version only implements a constructor and a "int handle(int event)" method. (I do not implement a destructor or clear() method).
The particular group that is crashing (they don't all seem to) includes widgets from Cartesian, a now apparently defunct 2D plotting library for FLTK developed by Roman Kantor.
The first entity that tries to double-free is a Ca_Y_Axis, which comes from Ca_Axis, which is derived from a Fl_Box.
The stack trace looks something like this...
- 0x103acdf20 Ca_Y_Axis::~Ca_Y_Axis
- 0x103c8b8d0 Fl_Group::clear
- 0x103c8ba84 Fl_Group::~Fl_Group
- 0x10263efe4 Vsp_Group::~Vsp_Group
- 0x103c8b8d0 Fl_Group::clear
- 0x103c8bb4c Fl_Group::~Fl_Group
- 0x103c8b8d0 Fl_Group::clear
- 0x103c8ba84 Fl_Group::~Fl_Group
- 0x10263efe4 Vsp_Group::~Vsp_Group
Is this a bug with the new Fl_Group::clear()?
if not, does anyone have suggestions for fixing it?
Do you expect it to be a problem with my subclass of Fl_Group, or a problem with Cartesian?
==> Fl_Cartesian.H <== // Cartesian.H,v 1.0 // // Copyright 2000-2005 by Roman Kantor. ==> Fl_Cartesian.cxx <== // Cartesian.cpp,v 1.0 // // Copyright 2000-2005 by Roman Kantor.Which version are you using, and if it's different (maybe newer), do you have a current URL for downloading it? I didn't find a newer version.
thanks for any ideas.
On 3/15/23 01:29 Rob McDonald wrote:
a09b0e2357f2 Remove reversal of children in Fl_Group::clear() (Albrecht Schlosser)
Causes my program to attempt to double-free a widget on shutdown.
[...]
The particular group that is crashing ... includes widgets from Cartesian, a [...] 2D plotting library for FLTK developed by Roman Kantor.
OK, after a little research I found OpenVSP on GitHub (which appears to be the project you mentioned) and a newer version of the Cartesian library here: https://sourceforge.net/p/rfltk/code/HEAD/tree/cartesian/
Meanwhile I also found out that this library adds more than one FLTK widget to its parent group (or window) - which may cause the double free under certain circumstances - and I could indeed make the provided example program crash with double free if I only delete the main window after `Fl::run()`.
Source code: https://sourceforge.net/p/rfltk/code/HEAD/tree/cartesian/test/example.cpp
Modified like this (see comment):
```
w->end();Fl_Double_Window *w = new Fl_Double_Window(580, 380, "Cartesian graphics example");// ... more code here ...w->show(argc, argv);Fl::add_timeout(0, next_freq);Fl::run();delete w; // ADDED only this statementreturn 0;```
This is an example I can work with, and I'll investigate this issue. Please don't expect quick results though, today my time is very much limited. I hope I can find something more tomorrow.
Current status of investigation:
1. the modified Cartesian example crashes with double free when built with FLTK 1.4.0 (git)
2. it does not crash with FLTK 1.3.x (not sure which git commit exactly), i.e. it exits silently
Note that this doesn't mean (yet) that the program doesn't *attempt* a double free in FLTK 1.3. So far it only means that this is *not diagnosed* by my current build of 1.3.x. I'll need to use a memory checker to verify this and I need more time to debug it.
I'll let you know when I have more info.
Sigh. I was afraid that this could happen but did it anyway (to improve speed) in the hope it wouldn't. It's difficult...
Given your stack trace below, which one is your subclass? I assume it is Vsp_Group, is this correct?
The particular group that is crashing (they don't all seem to) includes widgets from Cartesian, a now apparently defunct 2D plotting library for FLTK developed by Roman Kantor.OK, for my understanding, what do you mean with "now apparently defunct" ? Do you say this because your application crashes (with double free) or for any other reason?
Hard to say, but without seeing your code I suspect that your Vsp_Group *can* cause the issue. As you know, I'm responsible for the change that appears to cause it, and I have some ideas how this can happen. But to explain it I need more details, so ...
Is your code public, can I see it somewhere? Although it's maybe not a good idea to investigate in someone else's full code, I have some ideas what to look for, hence I could maybe try to do it. I would also be interested in testing the code so I could analyze it better. However, there shouldn't be too many dependencies.
That said, I wonder why I see 'Vsp_Group::~Vsp_Group' twice in the stack trace. Just to be sure, do you have nested Vsp_Group's, or is this maybe a recursion? Nested groups would be OK, a recursion probably not.
... I found ... a newer version of the Cartesian library here: https://sourceforge.net/p/rfltk/code/HEAD/tree/cartesian/
Meanwhile I also found out that this library adds more than one FLTK widget to its parent group (or window) - which may cause the double free under certain circumstances - and I could indeed make the provided example program crash with double free if I only delete the main window after `Fl::run()`.
Source code: https://sourceforge.net/p/rfltk/code/HEAD/tree/cartesian/test/example.cpp
Modified like this (see comment):
```
w->end();Fl_Double_Window *w = new Fl_Double_Window(580, 380, "Cartesian graphics example");// ... more code here ...w->show(argc, argv);Fl::add_timeout(0, next_freq);Fl::run();delete w; // ADDED only this statementreturn 0;```
This is an example I can work with, and I'll investigate this issue. ...
I'll let you know when I have more info.
Cartesian.cpp-728-Ca_X_Axis::~Ca_X_Axis(){ Cartesian.cpp-729- if(canvas_){ Cartesian.cpp-730- Ca_ObjectChain *ochain=canvas_->first_object_; Cartesian.cpp-731- Ca_ObjectChain *next; Cartesian.cpp-732- Ca_ObjectChain *previous=0; Cartesian.cpp-733- while (ochain){ Cartesian.cpp-734- next=ochain->next; Cartesian.cpp-735- if(ochain->object->x_axis_==this){ Cartesian.cpp-736- delete ochain->object; Cartesian.cpp-737- if(previous) Cartesian.cpp-738- previous->next=next; Cartesian.cpp-739- else Cartesian.cpp-740- canvas_->first_object_=next; Cartesian.cpp:741: // delete ochain; // would cause double free <<<<<<< Cartesian.cpp-742- } Cartesian.cpp-743- ochain=next; Cartesian.cpp-744- } Cartesian.cpp-745- } Cartesian.cpp-746-} -- Cartesian.cpp-1051-Ca_Y_Axis::~Ca_Y_Axis(){ Cartesian.cpp-1052- if(canvas_){ Cartesian.cpp-1053- Ca_ObjectChain *ochain=canvas_->first_object_; Cartesian.cpp-1054- Ca_ObjectChain *next; Cartesian.cpp-1055- Ca_ObjectChain *previous=0; Cartesian.cpp-1056- while (ochain){ Cartesian.cpp-1057- next=ochain->next; Cartesian.cpp-1058- if(ochain->object->y_axis_==this){ Cartesian.cpp-1059- delete ochain->object; Cartesian.cpp-1060- if(previous) Cartesian.cpp-1061- previous->next=next; Cartesian.cpp-1062- else Cartesian.cpp-1063- canvas_->first_object_=next; Cartesian.cpp:1064: // delete ochain; // would cause double free <<<<<<< Cartesian.cpp-1065- } Cartesian.cpp-1066- ochain=next; Cartesian.cpp-1067- } Cartesian.cpp-1068- } Cartesian.cpp-1069-}```
On Wednesday, March 15, 2023 at 7:12:16 AM UTC-7 Albrecht Schlosser wrote:
Sigh. I was afraid that this could happen but did it anyway (to improve speed) in the hope it wouldn't. It's difficult...
Understood. Sometimes we get lucky, sometimes we don't.
OK, for my understanding, what do you mean with "now apparently defunct" ? Do you say this because your application crashes (with double free) or for any other reason?
By 'now apparently defunct', I just meant that Cartesian is no longer a maintained / supported project. Its website is down and I believe the only references to it on the internet are some abandoned SourceForge pages.
We have occasionally had to make small changes to Cartesian to keep it going.
I think the rest of the questions are answered / superseded by your followup.
Thanks a ton for your attention to this and for all you do with FLTK.
Back on topic:
My proposed changes: since I'm not sure that we are using the same version of the Cartesian project, following is the full code of the two mentioned d'tors with fixes.
```
Cartesian.cpp-728-Ca_X_Axis::~Ca_X_Axis(){ Cartesian.cpp-729- if(canvas_){ Cartesian.cpp-730- Ca_ObjectChain *ochain=canvas_->first_object_; Cartesian.cpp-731- Ca_ObjectChain *next; Cartesian.cpp-732- Ca_ObjectChain *previous=0; Cartesian.cpp-733- while (ochain){ Cartesian.cpp-734- next=ochain->next; Cartesian.cpp-735- if(ochain->object->x_axis_==this){ Cartesian.cpp-736- delete ochain->object; Cartesian.cpp-737- if(previous) Cartesian.cpp-738- previous->next=next; Cartesian.cpp-739- else Cartesian.cpp-740- canvas_->first_object_=next; Cartesian.cpp:741: // delete ochain; // would cause double free <<<<<<< Cartesian.cpp-742- } Cartesian.cpp-743- ochain=next; Cartesian.cpp-744- } Cartesian.cpp-745- } Cartesian.cpp-746-} -- Cartesian.cpp-1051-Ca_Y_Axis::~Ca_Y_Axis(){ Cartesian.cpp-1052- if(canvas_){ Cartesian.cpp-1053- Ca_ObjectChain *ochain=canvas_->first_object_; Cartesian.cpp-1054- Ca_ObjectChain *next; Cartesian.cpp-1055- Ca_ObjectChain *previous=0; Cartesian.cpp-1056- while (ochain){ Cartesian.cpp-1057- next=ochain->next; Cartesian.cpp-1058- if(ochain->object->y_axis_==this){ Cartesian.cpp-1059- delete ochain->object; Cartesian.cpp-1060- if(previous) Cartesian.cpp-1061- previous->next=next; Cartesian.cpp-1062- else Cartesian.cpp-1063- canvas_->first_object_=next; Cartesian.cpp:1064: // delete ochain; // would cause double free <<<<<<< Cartesian.cpp-1065- } Cartesian.cpp-1066- ochain=next; Cartesian.cpp-1067- } Cartesian.cpp-1068- } Cartesian.cpp-1069-} ```
On Thursday, March 16, 2023 at 10:53:01 AM UTC-7 Albrecht Schlosser wrote:
Back on topic:
My proposed changes: since I'm not sure that we are using the same version of the Cartesian project, following is the full code of the two mentioned d'tors with fixes.
This fix works for me.
It took a while for me to figure out what he was doing -- it feels much more complex than a singly linked list should be. It took even longer to convince myself that the fix is right.
As you say -- it is hard to understand why the FLTK change would trigger this.
I also removed everything to do with previous -- it looks like a remnant of a doubly linked list, but it isn't used now.