wxruby development - Jul 2007 - Memory problem with GridCell*Editor

Alex,

I was getting the following crashes when editing cells in a grid:

./init.rb:1072:in `main_loop'': undefined method `begin_edit''
for
"_p_wxEvent":String (NoMethodError)
        from ./init.rb:1072

but only after the first App GC mark phase.

So I changed all the GridCell*Editor.i files from GC_MANAGE to
GC_MANAGE_AS_EVENT. Which stopped the crashing. My question Alex is do
you think this is the correct GC_* to use in this situation? I wanted
to run this by you before I do a commit, since I have not messed
around with memory management since the big overhaul.

thanks

Sean

Hey Sean

Sean Long wrote:
> I was getting the following crashes when editing cells in a grid:
>
> ./init.rb:1072:in `main_loop'': undefined method
`begin_edit'' for
> "_p_wxEvent":String (NoMethodError)
>         from ./init.rb:1072
>
> but only after the first App GC mark phase.
>   
Thanks - hmm -  looks like GC is getting called while an event is 
halfway through processing, then the event is getting re-wrapped inside 
a class that doesn''t know how to do the mapping correctly.

Unfortunately I can''t seem to reproduce this on Linux - is it on the 
grid.rb sample or in bigdemo?

If you could get a backtrace when the exception is raised that would be 
great. Try doing

gdb --args ruby -Ilib path/to/sample

then, in gdb, set a breakpoint when the exception is about to be raised

b rb_exc_raise

then ignore the first, false alarm (a load error) using

c

then when the real exception is raised, do

whe
> So I changed all the GridCell*Editor.i files from GC_MANAGE to
> GC_MANAGE_AS_EVENT. Which stopped the crashing. My question Alex is do
> you think this is the correct GC_* to use in this situation? 
The GC_MANAGE_AS_XXX declarations should align with the wx class 
hierarchy - they''re intended to match the lifecycle of different kind
of
wx objects - so really only classes derived from wxEvent should be 
GC_MANAGE_AS_EVENT. I think the grid editor classes should be 
GC_MANAGE_AS_OBJECT but obviously something''s going wrong.

Does it help if you make the GridCell*Editor classes GC_NEVER? If so, we 
could use this and just mark them FIXME. It''ll probably create a memory
leak, but only a small one.

cheers
alex

> Unfortunately I can''t seem to reproduce this on Linux - is it on
the
> grid.rb sample or in bigdemo?
Neither it is one of my own apps, which runs fine on 0.0.40.
> If you could get a backtrace when the exception is raised that would be
> great. Try doing
>
> gdb --args ruby -Ilib path/to/sample
>
> then, in gdb, set a breakpoint when the exception is about to be raised
>
> b rb_exc_raise
>
> then ignore the first, false alarm (a load error) using
>
> c
>
> then when the real exception is raised, do
>
> whe
I get:
No stack.
> Does it help if you make the GridCell*Editor classes GC_NEVER? If so, we
> could use this and just mark them FIXME. It''ll probably create a
memory
> leak, but only a small one.
GC_NEVER still causes a crash. The only GC_* that works is the
GC_MANAGE_AS_EVENT.

Any ideas?

Sean

Sean Long wrote:
>
> Neither it is one of my own apps, which runs fine on 0.0.40.
>   
Okay - does either of those exhibit the problem? Or is it easily 
reproducible in some small amount of code?
>
> GC_NEVER still causes a crash. The only GC_* that works is the
> GC_MANAGE_AS_EVENT.
The distinctive feature of manage_as_event is disabling of directors of 
object tracking. Does GC_MANAGE_AS_OBJECT plus either

%feature("nodirector") wxGridCellXXXEditor;

or

%trackobjects("0") # not sure how this should be switched off

work?

alex

Sean Long wrote:
> I was getting the following crashes when editing cells in a grid:
>
> ./init.rb:1072:in `main_loop'': undefined method
`begin_edit'' for
> "_p_wxEvent":String (NoMethodError)
>         from ./init.rb:1072
>
> but only after the first App GC mark phase.
>   
One more question - are you setting a custom GridCellEditor for your 
grid, and if so, what method are you using to assign it to the cells?

Does it then help if you store the Editor as a ruby instance variable of 
the Wx::Grid?

Thanks
alex

> The distinctive feature of manage_as_event is disabling of directors of
> object tracking. Does GC_MANAGE_AS_OBJECT plus either
>
> %feature("nodirector") wxGridCellXXXEditor;

Doing the above works with no crashing.

Any idea on why it works?

(sorry for the delayed response, my web connection was having problems
most of the day)

Sean

> One more question - are you setting a custom GridCellEditor for your
> grid, and if so, what method are you using to assign it to the cells?
I am setting the editor for an entire column. ex.

@grid =
Wx::Grid.new(panel,id_to_assign_grid,Wx::DEFAULT_POSITION,Wx::DEFAULT_SIZE,Wx::WANTS_CHARS|Wx::ALWAYS_SHOW_SB)
@grid.create_grid(4,3)

float_cell_attr
Wx::GridCellAttr.new(Wx::BLACK,Wx::WHITE,Wx::NORMAL_FONT,Wx::ALIGN_CENTRE,Wx::ALIGN_CENTRE)
			
float_cell_attr.set_editor(Wx::GridCellFloatEditor.new(2,1))
@grid.set_col_attr(1,float_cell_attr)
> Does it then help if you store the Editor as a ruby instance variable of
> the Wx::Grid?
Have not tried this yet.


Sean

Sean Long wrote:
>> %feature("nodirector") wxGridCellXXXEditor;
>>     
>
>
> Doing the above works with no crashing.
>   
Thanks - from this and your other email I''ve a pretty good guess at 
what''s going on. I can reproduce it and have got some ideas to fix it
to
try out later.

A bit of background:

Classes with directors are more sensitive to getting GC''d prematurely, 
and are one of the biggest headaches in wxRuby. This is because in 
SWIG''s design  the *same* ruby object is permanently bound to the C++ 
object - so that calls to C++ virtual methods from inside the wxWidget 
API can be delegated to ruby subclass methods.

Because of this, director classes (GridCell*Editor and 
GridCell*Renderer), which don''t inherit from Wx::Window but are 
attached/owned by windows and have a finite lifespan need special GC 
treatment by the owner. Otherwise, when GC runs, it finds that the ruby 
GridCell*Editor object is no longer referenced anywhere in ruby, sweeps 
the object, and then subsequently when a C++ virtual method call is made 
(eg ''begin_edit''), it tries to call it on a deleted ruby obj,
which
either crashes or gives strange results like the ones you were seeing. 
The same sort of problem applies to Menus, Sizers, ClientData, ToolTips etc.

One way to fix this is to make the Editor (and Renderer) classes not 
directors. But this would meant that it would not be possible to write 
your own Editor or Renderer classes in Ruby because the relevant 
internal methods (mostly in the base class GridCellEditor) couldn''t be 
over-ridden. That''s maybe something only pretty advanced users will
want
to do, but still nice to have.

The better way to fix it would be to improve the way Grid is marked in 
the GC phase so it marks all associated editors. The way I currently 
implemented this (in lib/wx/classes/grid.rb) missed the fact that an 
editor could be attached via attr and set_col_attr, as well as directly 
by set_cell_editor.

cheers
alex

Hi Sean

Alex Fenton wrote:
> Sean Long wrote:
>   
>>> %feature("nodirector") wxGridCellXXXEditor;
>>>     
>>>       
>> Doing the above works with no crashing.
>>     
I''ve checked in some code that should help the original issue. 
Unfortunately 1) it''s a bit complicated and 2) the performance sucks on
larger grids. 20x20 is fine, but 200x200 is not - it seizes up when GC runs.

The choice as far as I can see for now is to either

- remove directors from GridCellEditor and GridCellRenderer; this means 
users can''t write their own custom ruby DC code to paint directly on a 
cell when it''s being edited or re-drawn. The standard Wx variants (eg 
Wx::GridCellFloatEditor) would still work

or

- warn users that they can''t create more than modest-sized grids.

What do you (or others) think?

cheers
alex

> The choice as far as I can see for now is to either
>
> - remove directors from GridCellEditor and GridCellRenderer; this means
> users can''t write their own custom ruby DC code to paint directly
on a
> cell when it''s being edited or re-drawn. The standard Wx variants
(eg
> Wx::GridCellFloatEditor) would still work
>
> or
>
> - warn users that they can''t create more than modest-sized grids.
>
> What do you (or others) think?
I think limiting the grid size would be a bad idea, in one of my
programs I use a grid with 10 columns and over 100 rows. I can not
remember ever having a need to create my own drawing code for cell
editors and cell renderers.

So I guess I am for removing directors for now until some other
solution presents itself that does not compromise the size of the
grids.

Sean

Sean Long wrote:
> I think limiting the grid size would be a bad idea, in one of my
> programs I use a grid with 10 columns and over 100 rows. I can not
> remember ever having a need to create my own drawing code for cell
> editors and cell renderers.
>
> So I guess I am for removing directors for now until some other
> solution presents itself that does not compromise the size of the
> grids.
OK, I''m of the same view, though it''s not ideal.

It could probably be fixed by writing a whole C++ subclass of wxGrid 
(wxRubyGrid) that does more efficient tracking of the diretor classes, 
and then wrapping it as Wx::Grid as is done in wxApp and WxArtProvider. 
But it''s a hefty task and I don''t think it''s
important enough right now.

If nothing more comes up I''ll plan to tag over the weekend.

cheers
alex

Sean Long wrote:
> I think limiting the grid size would be a bad idea, in one of my
> programs I use a grid with 10 columns and over 100 rows. I can not
> remember ever having a need to create my own drawing code for cell
> editors and cell renderers.
Removing Directors produced other undesirable behaviour and at the same 
time I thought of a better Ruby algo for handling cell editors. Good 
Ruby is still much faster than bad C++ and it seems to be OK with 
2000x2000 Grids.

I tried a few scenarios calling App#gc_stress in on_init and it seems to 
be stable, but let me know if there are still probs

alex