| | [ Return to Bugs & Features | Roadmap 1.3 | SVN ⇄ GIT ]
STR #3427
| Application: | FLTK Library |
|---|
| Status: | 1 - Closed w/Resolution |
|---|
| Priority: | 3 - Moderate, e.g. unable to compile the software |
|---|
| Scope: | 2 - Specific to an operating system |
|---|
| Subsystem: | FLUID |
|---|
| Summary: | Fluid app crashes on exit when fl file has Fl_Table and Fl_Tree widgets |
|---|
| Version: | 1.3.4 |
|---|
| Created By: | Nikego |
|---|
| Assigned To: | AlbrechtS |
|---|
| Fix Version: | 1.3.5 (SVN: v12567) |
|---|
| Update Notification: | |
|---|
Trouble Report Files:
Trouble Report Comments:
| |
| #1 | Nikego
11:45 Nov 04, 2017 |
| MS Windows7 64
VisualStudio 2015
I tested a few last versions (including release 1.3.4) and they have the same problem - crash on exit or in other unpredictable moments. I have investigated the problem and created simplest fl file having this effect in Fluid app. There'is only two widgets Fl_Table and Fl_Tree, order of sequence is important.
Fluid throws an exception with message, when I'm trying to close it:
read access violation.
this->o-> was 0xDDDDDDDD
Debugger is stopped at line 212 in Fl_Widget_Type.cxx (it's destructor of class Fl_Widget_Type).
I suppose Fluid attempts to release an object, that is already freed. | |
| |
| #2 | greg.ercolano
01:26 Nov 06, 2017 |
| I poked at this a little; I don't know fluid's internals.
1) I can replicate in linux with the same thing
2) From what I can tell, the crash happens in the dtor
of one of the widgets created when fluid starts (before it loads
anything)
3) To get Fl_Table and Fl_Tree's internals out of the equation,
I replaced all instancing of Fl_Table + Fl_Tree in fluid
with instancing an Fl_Group instead, and was still getting
the crash, so I don't think those widget's own code has
anything to do with the crash.
Although I authored Fl_Tree/Fl_Table, I didn't add them to fluid,
and I'm not at all familiar with fluid's internals, which is tricky.
Awaiting input from an FLTK dev who knows fluid's internals.. | |
| |
| #3 | AlbrechtS
03:58 Nov 06, 2017 |
| Hmm, I also tested (FLTK 1.4.0) with Nikita's fl file and I can reproduce the crash.
I /couldn't/ reproduce the crash when I replaced Fl_Table and Fl_Tree with Fl_Group. See uploaded fluid file group.fl. Greg, is your file different? If yes, can you please post it?
I can confirm that the crash happens in the d'tor of a static variable (Fl_Tree_type) of class Fl_Tree_Type (note different case of '[t|T]ype') in file fluid/factory.cxx, line #303:
static Fl_Tree_Type Fl_Tree_type;
The crash happens in
Fl_Widget_Type::~Fl_Widget_Type() {
fprintf(stderr, "d'tor Fl_Widget_Type [%3d]: %p\n", nwt--, this);
if (o) {
o->hide();
if (o->parent()) ((Fl_Group*)o->parent())->remove(*o);
delete o; // crashes here
}
...
}
Since o is an Fl_Widget* I suppose that the widget itself (i.e. Fl_Tree) has already been deleted at this point, but that's only a guess.
Sorry, I can't test further right now... | |
| |
| #4 | AlbrechtS
04:00 Nov 06, 2017 |
| Note: I added some printf's to the code, the above code fragment contains one of my printf() test statements. | |
| |
| #5 | Nikego
09:30 Nov 06, 2017 |
| Greg wrote:
> I replaced all instancing of Fl_Table + Fl_Tree in fluid
with instancing an Fl_Group instead, and was still getting
the crash, so I don't think those widget's own code has
anything to do with the crash.
Greg, I'm afraid you just replaced Fl_Table + Fl_Tree and tryed to exit. It doesn't work: old widgets stayed in memory. You have to save your changes into fl file, then restart Fluid with the saved file. Only then you can test for crash. | |
| |
| #6 | AlbrechtS
09:19 Nov 08, 2017 |
| Please try uploaded patch fix_double_delete.patch and report if this fixes the issue for you. Your feedback will help to dig further into the issue to hopefully find a final patch.
This patch is for FLTK 1.4.0 (not 1.3.x, but may be usable for 1.3.x as well or with minor tweaks). It was tested only under Linux and verified with valgrind.
Note: this is a first test and supposedly not the final patch. It kinda "cures the symptoms" but seems to work well for me.
Rationale: Fluid keeps its own objects of class Fl_Type, Fl_Widget_Type, and other derived classes. These classes can contain pointers to widgets, and the current code deletes these widgets when the Fl_Widget_Type (or other) object is destroyed, which happens at exit for some static variables. The crux is that widgets are also added to instances of Fl_Group widgets used in the fluid design. If such an Fl_Group widget (like, in the given case Fl_Tree or Fl_Table) that contains other widgets is deleted, then all children are deleted as well - but the "management" object (Fl_Widget_Type etc.) is not notified of this fact. Hence it will try to delete the widget a second time which leads to the crash. The solution in this patch is to use an Fl_Widget_Tracker object to know if the widget was already deleted when the Fl_Widget_Type object gets destroyed.
I'm not sure if there is a better way to deal with this. I saw that Fl_Tree and (maybe) Fl_Table widgets are initialized with some dummy contents which may have caused the issue in the first place. This needs further investigation...
Side note: use valgrind to see lots of warnings before applying the patch. Almost all warnings are gone with the patch; only one benign X11 specific warning remained in my test with table-tree.fl. Test procedure: run 'fluid table-tree.fl' and exit fluid immediately w/o changing anything. | |
| |
| #7 | greg.ercolano
09:21 Nov 08, 2017 |
| @Nikego: Sorry for the confusion: I meant I removed Fl_Table+Fl_Tree
*from fluid's own source code* so that the application fluid
was built without referring to Fl_Table or Fl_Tree internally,
and just used an Fl_Group instead.
When I then loaded your .fl file, fluid still crashed, even though
internally fluid wasn't actually using the Fl_Table/Tree widgets,
and just used an Fl_Group instead.
I just wanted to make sure the bug was in fluid, and not one of the
widgets, as they are both relatively "new" widgets.
By removing them from fluid's source code and replacing them with
Fl_Group ruled out this being a bug in either of those two widgets'
own code, confirming the bug somewhere in fluid itself. | |
| |
| #8 | AlbrechtS
09:23 Nov 08, 2017 |
| Assigned to myself, set "Active" status and subsystem "fluid". | |
| |
| #9 | Nikego
12:58 Nov 08, 2017 |
| Thanks, Albrecht! I've patched my working copy of FLTK and played with Fluid: so far so good. I'll write here in case of new troubles. | |
| |
| #10 | Nikego
14:14 Nov 13, 2017 |
| Hello, I'm here again, unfortunately the Fluid continues to crash. Now it crashes when I'm trying to delete a window: it occurs with our fl file too. Previous version I tested has no problem.
Second: Undo/Redo create new windows, every action makes new window. It's new feature too.
I suppose combination of those two troubles makes one more crash of the Fluid when I'm working with my fl file (it's too big to put here) and trying to press Ctrl-Z.
On the other hand the previous version of the Fluid too crashes here, so ... | |
| |
| #11 | AlbrechtS
04:54 Nov 14, 2017 |
| To the crashes: my attempt to fix it (the posted patch) was somewhat like a shot in the dark; I didn't analyze the entire fluid code. But I had a feeling that we need a major rewrite or at least a change in the strategy of deleting widgets. I'll take another look at it later.
> Second: Undo/Redo create new windows, every action makes new window.
> It's new feature too.
Is this "new feature" ;-) only with the patch or is it generally in current svn (FLTK 1.4) or even in 1.3.4? Or in which exact version did you see it? | |
| |
| #12 | Nikego
05:14 Nov 14, 2017 |
| > my attempt to fix it (the posted patch) was somewhat like a shot in the dark;
Well, no problem. You tryed to fix, I tested. Thanks for your efforts.
> Is this "new feature" only with the patch or is it generally in current
svn (FLTK 1.4) or even in 1.3.4? Or in which exact version did you see it?
I haven't 1.4 on my computers at all. Only 1.3.4. I wrote it in the description of this STR. | |
| |
| #13 | chris
08:21 Nov 17, 2017 |
| Solved!
There's an end() missing:
Index: Fl_Group_Type.cxx
===================================================================
--- Fl_Group_Type.cxx (Revision 12562)
+++ Fl_Group_Type.cxx (Arbeitskopie)
@@ -196,6 +196,7 @@
public:
Fluid_Table(int x, int y, int w, int h, const char *l=0L)
: Fl_Table(x, y, w, h, l) {
+ end();
for ( int r=0; r<MAX_ROWS; r++ )
for ( int c=0; c<MAX_COLS; c++ )
data[r][c] = 1000+(r*1000)+c;
This is necessary because Fl_Table() constructor ends with (excerpt from Fl_Table.cxx from line 196):
Fl_Group::end(); // end the group's begin()
table->begin(); // leave with fltk children getting added to the scroll
} | |
| |
| #14 | AlbrechtS
10:30 Nov 17, 2017 |
| Awesome! Thanks for the patch!
Chris, did you apply your patch together with mine, or do you think your patch alone solves the issue? I didn't test it yet, just wanted to ask before doing unnecessary work. | |
| |
| #15 | chris
10:35 Nov 17, 2017 |
| No, just this single line change. | |
| |
| #16 | Nikego
10:50 Nov 17, 2017 |
| Thank you, chris! You really solved the problem! I removed Albrecht's patch and all works fine. Even my big fl file now works without crashes. Hope, there's no new bugs :) | |
| |
| #17 | AlbrechtS
10:29 Nov 18, 2017 |
| Fixed in Subversion repository.
Note: I found another related bug while I was testing with valgrind: valgrind reported:
Invalid read of size 4
at 0x46634C: Fl_Widget::visible() const (in /../bin/fluid)
by 0x49C416: update_sourceview_position() (fluid.cxx:1593)
I fixed this one as well in another commit in FLTK 1.3.5 and 1.4.0.
Commits in numerical order:
r12564 (1.3.5): Fix crash when closing fluid with Fl_Table (STR #3427).
r12565 (1.4.0): Fix crash when closing fluid with Fl_Table (STR #3427).
r12566 (1.3.5): Fix illegal memory access after free ... (STR #3427).
r12567 (1.4.0): Fix illegal memory access after free ... (STR #3427).
Neither 1.3.5 nor 1.4.0 are currently released though.
Thanks again to Chris for the good patch!
@Nikita: I'll leave this STR open for a few days for your confirmation that svn r12566/12567 doesn't break anything in your environment. TIA. | |
| |
| #18 | AlbrechtS
12:47 Nov 26, 2017 |
| Closed. | |
[ Return to Bugs & Features ]
|
| |
