From 7ae0c170ad2f63ac7e27d0496cd9c167e2bd0cbd Mon Sep 17 00:00:00 2001 From: Albrecht Schlosser Date: Sat, 20 Feb 2010 17:40:07 +0000 Subject: Added Fl_Widget_Tracker in handle() methods etc. to avoid accessing widgets after deletion (STR #1306). This is all I could find, but maybe there are more places in the code. git-svn-id: file:///fltk/svn/fltk/branches/branch-1.3@7115 ea41ed52-d2ee-0310-a9c1-e6b18d33e121 --- src/Fl.cxx | 16 +++++------ src/Fl_Adjuster.cxx | 20 +++++++++----- src/Fl_Browser_.cxx | 56 ++++++++++++++++++++++++++++++++++----- src/Fl_Button.cxx | 8 +++++- src/Fl_Counter.cxx | 7 ++++- src/Fl_Dial.cxx | 4 ++- src/Fl_File_Input.cxx | 9 ++++--- src/Fl_Group.cxx | 3 ++- src/Fl_Slider.cxx | 72 ++++++++++++++++++++++++++++++-------------------- src/Fl_Tabs.cxx | 2 ++ src/Fl_Value_Input.cxx | 4 ++- 11 files changed, 143 insertions(+), 58 deletions(-) (limited to 'src') diff --git a/src/Fl.cxx b/src/Fl.cxx index c8b359c6b..9670e29c2 100644 --- a/src/Fl.cxx +++ b/src/Fl.cxx @@ -44,9 +44,9 @@ #import #endif -#ifdef DEBUG +#if defined(DEBUG) || defined(DEBUG_WATCH) # include -#endif // DEBUG +#endif // DEBUG || DEBUG_WATCH #ifdef WIN32 # include @@ -1648,11 +1648,11 @@ void Fl::watch_widget_pointer(Fl_Widget *&w) widget_watch = (Fl_Widget***)realloc(widget_watch, sizeof(Fl_Widget**)*max_widget_watch); } widget_watch[num_widget_watch++] = wp; -#ifdef DEBUG +#ifdef DEBUG_WATCH printf ("\nwatch_widget_pointer: (%d/%d) %8p => %8p\n", num_widget_watch,num_widget_watch,wp,*wp); fflush(stdout); -#endif // DEBUG +#endif // DEBUG_WATCH } /** @@ -1674,18 +1674,18 @@ void Fl::release_widget_pointer(Fl_Widget *&w) if (j %8p\n", i+1,num_widget_watch,wp,*wp); } -#endif //DEBUG +#endif //DEBUG_WATCH } num_widget_watch = j; -#ifdef DEBUG +#ifdef DEBUG_WATCH printf (" num_widget_watch = %d\n\n",num_widget_watch); fflush(stdout); -#endif // DEBUG +#endif // DEBUG_WATCH return; } /** diff --git a/src/Fl_Adjuster.cxx b/src/Fl_Adjuster.cxx index 6a7d06060..4f14c7a39 100644 --- a/src/Fl_Adjuster.cxx +++ b/src/Fl_Adjuster.cxx @@ -69,6 +69,7 @@ int Fl_Adjuster::handle(int event) { double v; int delta; int mx = Fl::event_x(); + // Fl_Widget_Tracker wp(this); switch (event) { case FL_PUSH: if (Fl::visible_focus()) Fl::focus(this); @@ -77,7 +78,10 @@ int Fl_Adjuster::handle(int event) { drag = 3*(mx-x())/w() + 1; else drag = 3-3*(Fl::event_y()-y()-1)/h(); - handle_push(); + { Fl_Widget_Tracker wp(this); + handle_push(); + if (wp.deleted()) return 1; + } redraw(); return 1; case FL_DRAG: @@ -98,9 +102,9 @@ int Fl_Adjuster::handle(int event) { delta = 0; } switch (drag) { - case 3: v = increment(previous_value(), delta); break; - case 2: v = increment(previous_value(), delta*10); break; - default:v = increment(previous_value(), delta*100); break; + case 3: v = increment(previous_value(), delta); break; + case 2: v = increment(previous_value(), delta*10); break; + default:v = increment(previous_value(), delta*100); break; } handle_drag(soft() ? softclamp(v) : clamp(v)); return 1; @@ -109,11 +113,13 @@ int Fl_Adjuster::handle(int event) { if (Fl::event_state()&0xF0000) delta = -10; else delta = 10; switch (drag) { - case 3: v = increment(previous_value(), delta); break; - case 2: v = increment(previous_value(), delta*10); break; - default:v = increment(previous_value(), delta*100); break; + case 3: v = increment(previous_value(), delta); break; + case 2: v = increment(previous_value(), delta*10); break; + default:v = increment(previous_value(), delta*100); break; } + Fl_Widget_Tracker wp(this); handle_drag(soft() ? softclamp(v) : clamp(v)); + if (wp.deleted()) return 1; } drag = 0; redraw(); diff --git a/src/Fl_Browser_.cxx b/src/Fl_Browser_.cxx index 2a654dbe5..ad98f858f 100644 --- a/src/Fl_Browser_.cxx +++ b/src/Fl_Browser_.cxx @@ -678,11 +678,15 @@ int Fl_Browser_::deselect(int docallbacks) { int Fl_Browser_::select_only(void* item, int docallbacks) { if (!item) return deselect(docallbacks); int change = 0; + Fl_Widget_Tracker wp(this); if (type() == FL_MULTI_BROWSER) { - for (void* p = item_first(); p; p = item_next(p)) + for (void* p = item_first(); p; p = item_next(p)) { if (p != item) change |= select(p, 0, docallbacks); + if (wp.deleted()) return change; + } } change |= select(item, 1, docallbacks); + if (wp.deleted()) return change; display(item); return change; } @@ -693,6 +697,20 @@ int Fl_Browser_::select_only(void* item, int docallbacks) { \returns 1 if event was processed, 0 if not. */ int Fl_Browser_::handle(int event) { + + // NOTE: + // We use Fl_Widget_Tracker to test if the user has deleted + // this widget in a callback. Callbacks can be called by: + // - do_callback() + // - select() + // - select_only() + // - deselect() + // Thus we must test wp.deleted() after each of these calls, + // unless we return directly after one of these. + // If wp.deleted() is true, we return 1 because we used the event. + + Fl_Widget_Tracker wp(this); + // must do shortcuts first or the scrollbar will get them... if (event == FL_ENTER || event == FL_LEAVE) return 1; if (event == FL_KEYBOARD && type() >= FL_HOLD_BROWSER) { @@ -706,8 +724,12 @@ int Fl_Browser_::handle(int event) { if (item_height(l)>0) {select_only(l, when()); break;} return 1; case FL_Up: - while ((l = item_prev(l))) if (item_height(l)>0) { - select_only(l, when()); break;} + while ((l = item_prev(l))) { + if (item_height(l)>0) { + select_only(l, when()); + break; // no need to test wp (return 1) + } + } return 1; } } else { @@ -715,6 +737,7 @@ int Fl_Browser_::handle(int event) { case FL_Enter: case FL_KP_Enter: select_only(l, when() & ~FL_WHEN_ENTER_KEY); + if (wp.deleted()) return 1; if (when() & FL_WHEN_ENTER_KEY) { set_changed(); do_callback(); @@ -728,6 +751,7 @@ int Fl_Browser_::handle(int event) { while ((l = item_next(l))) { if (Fl::event_state(FL_SHIFT|FL_CTRL)) select(l, l1 ? item_selected(l1) : 1, when()); + if (wp.deleted()) return 1; if (item_height(l)>0) goto J1; } return 1; @@ -735,6 +759,7 @@ int Fl_Browser_::handle(int event) { while ((l = item_prev(l))) { if (Fl::event_state(FL_SHIFT|FL_CTRL)) select(l, l1 ? item_selected(l1) : 1, when()); + if (wp.deleted()) return 1; if (item_height(l)>0) goto J1; } return 1; @@ -749,6 +774,8 @@ J1: } if (Fl_Group::handle(event)) return 1; + if (wp.deleted()) return 1; + int X, Y, W, H; bbox(X, Y, W, H); int my; // NOTE: @@ -784,9 +811,11 @@ J1: ; else if (type() != FL_MULTI_BROWSER) { change = select_only(find_item(my), 0); + if (wp.deleted()) return 1; if (change && (when() & FL_WHEN_CHANGED)) { set_changed(); do_callback(); + if (wp.deleted()) return 1; } } else { void* l = find_item(my); @@ -796,9 +825,11 @@ J1: if (l) { whichway = !item_selected(l); change = select(l, whichway, 0); + if (wp.deleted()) return 1; if (change && (when() & FL_WHEN_CHANGED)) { set_changed(); do_callback(); + if (wp.deleted()) return 1; } } } else if (Fl::event_state(FL_SHIFT)) { // extend selection: @@ -814,23 +845,29 @@ J1: if (!m) {down = 0; break;} }} if (down) { - for (void* m = selection_; m != l; m = item_next(m)) + for (void* m = selection_; m != l; m = item_next(m)) { select(m, whichway, when() & FL_WHEN_CHANGED); + if (wp.deleted()) return 1; + } } else { void* e = selection_; for (void* m = item_next(l); m; m = item_next(m)) { select(m, whichway, when() & FL_WHEN_CHANGED); + if (wp.deleted()) return 1; if (m == e) break; } } // do the clicked item last so the select box is around it: change = 1; if (l) select(l, whichway, when() & FL_WHEN_CHANGED); + if (wp.deleted()) return 1; } else { // select only this item change = select_only(l, 0); + if (wp.deleted()) return 1; if (change && (when() & FL_WHEN_CHANGED)) { set_changed(); do_callback(); + if (wp.deleted()) return 1; } } } @@ -863,10 +900,12 @@ J1: for (; t && t != b; t = item_next(t)) { char change_t; change_t = select(t, whichway, 0); + if (wp.deleted()) return 1; change |= change_t; if (change_t && (when() & FL_WHEN_CHANGED)) { set_changed(); do_callback(); + if (wp.deleted()) return 1; } } if (l) selection_ = l; @@ -877,12 +916,16 @@ J1: find_item(my); change = (l != l1); select_only(l, when() & FL_WHEN_CHANGED); + if (wp.deleted()) return 1; } py = my; return 1; case FL_RELEASE: if (type() == FL_SELECT_BROWSER) { - void* t = selection_; deselect(); selection_ = t; + void* t = selection_; + deselect(); + if (wp.deleted()) return 1; + selection_ = t; } if (change) { set_changed(); @@ -890,7 +933,8 @@ J1: } else { if (when() & FL_WHEN_NOT_CHANGED) do_callback(); } - + if (wp.deleted()) return 1; + // double click calls the callback: (like Enter Key) if (Fl::event_clicks() && (when() & FL_WHEN_ENTER_KEY)) { set_changed(); diff --git a/src/Fl_Button.cxx b/src/Fl_Button.cxx index c32075999..147e5cf4d 100644 --- a/src/Fl_Button.cxx +++ b/src/Fl_Button.cxx @@ -117,7 +117,11 @@ int Fl_Button::handle(int event) { else { value(oldval); set_changed(); - if (when() & FL_WHEN_CHANGED) do_callback(); + if (when() & FL_WHEN_CHANGED) { + Fl_Widget_Tracker wp(this); + do_callback(); + if (wp.deleted()) return 1; + } } if (when() & FL_WHEN_RELEASE) do_callback(); return 1; @@ -156,6 +160,7 @@ int Fl_Button::handle(int event) { if (Fl::focus() == this && Fl::event_key() == ' ' && !(Fl::event_state() & (FL_SHIFT | FL_CTRL | FL_ALT | FL_META))) { set_changed(); + Fl_Widget_Tracker wp(this); if (type() == FL_RADIO_BUTTON && !value_) { setonly(); if (when() & FL_WHEN_CHANGED) do_callback(); @@ -163,6 +168,7 @@ int Fl_Button::handle(int event) { value(!value()); if (when() & FL_WHEN_CHANGED) do_callback(); } + if (wp.deleted()) return 1; if (when() & FL_WHEN_RELEASE) do_callback(); return 1; } diff --git a/src/Fl_Counter.cxx b/src/Fl_Counter.cxx index b77fc8a4c..de83f15d5 100644 --- a/src/Fl_Counter.cxx +++ b/src/Fl_Counter.cxx @@ -137,14 +137,19 @@ int Fl_Counter::handle(int event) { return 1; case FL_PUSH: if (Fl::visible_focus()) Fl::focus(this); - handle_push(); + { Fl_Widget_Tracker wp(this); + handle_push(); + if (wp.deleted()) return 1; + } case FL_DRAG: i = calc_mouseobj(); if (i != mouseobj) { Fl::remove_timeout(repeat_callback, this); mouseobj = (uchar)i; if (i) Fl::add_timeout(INITIALREPEAT, repeat_callback, this); + Fl_Widget_Tracker wp(this); increment_cb(); + if (wp.deleted()) return 1; redraw(); } return 1; diff --git a/src/Fl_Dial.cxx b/src/Fl_Dial.cxx index f91b55df9..fed278ca3 100644 --- a/src/Fl_Dial.cxx +++ b/src/Fl_Dial.cxx @@ -109,8 +109,10 @@ void Fl_Dial::draw() { */ int Fl_Dial::handle(int event, int X, int Y, int W, int H) { switch (event) { - case FL_PUSH: + case FL_PUSH: { + Fl_Widget_Tracker wp(this); handle_push(); + if (wp.deleted()) return 1; } case FL_DRAG: { int mx = (Fl::event_x()-X-W/2)*H; int my = (Fl::event_y()-Y-H/2)*W; diff --git a/src/Fl_File_Input.cxx b/src/Fl_File_Input.cxx index efd2d062e..8faf8b0bd 100644 --- a/src/Fl_File_Input.cxx +++ b/src/Fl_File_Input.cxx @@ -215,9 +215,12 @@ Fl_File_Input::handle(int event) // I - Event return Fl_Input::handle(event); default : - if (Fl_Input::handle(event)) { - damage(FL_DAMAGE_BAR); - return 1; + { Fl_Widget_Tracker wp(this); + if (Fl_Input::handle(event)) { + if (wp.exists()) + damage(FL_DAMAGE_BAR); + return 1; + } } return 0; } diff --git a/src/Fl_Group.cxx b/src/Fl_Group.cxx index b42f1caf8..4deba31ce 100644 --- a/src/Fl_Group.cxx +++ b/src/Fl_Group.cxx @@ -227,8 +227,9 @@ int Fl_Group::handle(int event) { for (i = children(); i--;) { o = a[i]; if (o->takesevents() && Fl::event_inside(o)) { + Fl_Widget_Tracker wp(o); if (send(o,FL_PUSH)) { - if (Fl::pushed() && !o->contains(Fl::pushed())) Fl::pushed(o); + if (Fl::pushed() && wp.exists() && !o->contains(Fl::pushed())) Fl::pushed(o); return 1; } } diff --git a/src/Fl_Slider.cxx b/src/Fl_Slider.cxx index 93a4457fd..6b2a328d1 100644 --- a/src/Fl_Slider.cxx +++ b/src/Fl_Slider.cxx @@ -222,10 +222,14 @@ void Fl_Slider::draw() { } int Fl_Slider::handle(int event, int X, int Y, int W, int H) { + // Fl_Widget_Tracker wp(this); switch (event) { - case FL_PUSH: + case FL_PUSH: { + Fl_Widget_Tracker wp(this); if (!Fl::event_inside(X, Y, W, H)) return 0; handle_push(); + if (wp.deleted()) return 1; } + // fall through ... case FL_DRAG: { double val; @@ -293,34 +297,44 @@ int Fl_Slider::handle(int event, int X, int Y, int W, int H) { case FL_RELEASE: handle_release(); return 1; - case FL_KEYBOARD : - switch (Fl::event_key()) { - case FL_Up: - if (horizontal()) return 0; - handle_push(); - handle_drag(clamp(increment(value(),-1))); - handle_release(); - return 1; - case FL_Down: - if (horizontal()) return 0; - handle_push(); - handle_drag(clamp(increment(value(),1))); - handle_release(); - return 1; - case FL_Left: - if (!horizontal()) return 0; - handle_push(); - handle_drag(clamp(increment(value(),-1))); - handle_release(); - return 1; - case FL_Right: - if (!horizontal()) return 0; - handle_push(); - handle_drag(clamp(increment(value(),1))); - handle_release(); - return 1; - default: - return 0; + case FL_KEYBOARD: + { Fl_Widget_Tracker wp(this); + switch (Fl::event_key()) { + case FL_Up: + if (horizontal()) return 0; + handle_push(); + if (wp.deleted()) return 1; + handle_drag(clamp(increment(value(),-1))); + if (wp.deleted()) return 1; + handle_release(); + return 1; + case FL_Down: + if (horizontal()) return 0; + handle_push(); + if (wp.deleted()) return 1; + handle_drag(clamp(increment(value(),1))); + if (wp.deleted()) return 1; + handle_release(); + return 1; + case FL_Left: + if (!horizontal()) return 0; + handle_push(); + if (wp.deleted()) return 1; + handle_drag(clamp(increment(value(),-1))); + if (wp.deleted()) return 1; + handle_release(); + return 1; + case FL_Right: + if (!horizontal()) return 0; + handle_push(); + if (wp.deleted()) return 1; + handle_drag(clamp(increment(value(),1))); + if (wp.deleted()) return 1; + handle_release(); + return 1; + default: + return 0; + } } // break not required because of switch... case FL_FOCUS : diff --git a/src/Fl_Tabs.cxx b/src/Fl_Tabs.cxx index 41de89a7e..e36a4cbda 100644 --- a/src/Fl_Tabs.cxx +++ b/src/Fl_Tabs.cxx @@ -161,8 +161,10 @@ int Fl_Tabs::handle(int event) { redraw_tabs(); } if (o && value(o)) { + Fl_Widget_Tracker wp(o); set_changed(); do_callback(); + if (wp.deleted()) return 1; } Fl_Tooltip::current(o); } else { diff --git a/src/Fl_Value_Input.cxx b/src/Fl_Value_Input.cxx index 60f204eb4..7b804225d 100644 --- a/src/Fl_Value_Input.cxx +++ b/src/Fl_Value_Input.cxx @@ -100,8 +100,10 @@ int Fl_Value_Input::handle(int event) { if (value() != previous_value() || !Fl::event_is_click()) handle_release(); else { + Fl_Widget_Tracker wp(&input); input.handle(FL_PUSH); - input.handle(FL_RELEASE); + if (wp.exists()) + input.handle(FL_RELEASE); } return 1; case FL_FOCUS: -- cgit v1.2.3