On Thu, 30 Jan 2014 11:03:48 +0900 Carsten Haitzler (The Rasterman) <ras...@rasterman.com> wrote:
> On Wed, 29 Jan 2014 20:39:41 -0500 Michael Blumenkrantz > <michael.blumenkra...@gmail.com> said: > > > On Thu, 30 Jan 2014 10:32:26 +0900 > > Carsten Haitzler (The Rasterman) <ras...@rasterman.com> wrote: > > > > > On Wed, 29 Jan 2014 20:23:46 -0500 Michael Blumenkrantz > > > <michael.blumenkra...@gmail.com> said: > > > > > > > On Wed, 29 Jan 2014 16:01:10 -0800 > > > > Carsten Haitzler <ras...@rasterman.com> wrote: > > > > > > > > > raster pushed a commit to branch master. > > > > > > > > > > http://git.enlightenment.org/core/enlightenment.git/commit/?id=83397e1bde51830016e9a0f8e6482fc91bb4c50c > > > > > > > > > > commit 83397e1bde51830016e9a0f8e6482fc91bb4c50c > > > > > Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com> > > > > > Date: Thu Jan 30 08:55:28 2014 +0900 > > > > > > > > > > fix segv where comp_data is null but still accessed > > > > > > > > > > it seems i have an override-redirect window just off the > > > > > bottom-right of my screen - i think its the scim input panel status. > > > > > what happens is it is "managed" by comp but then deleted > > > > > (_e_comp_x_hook_client_del called), BUT _e_comp_x_object_add is called > > > > > with a deferred event for that client to add it again (likely this is > > > > > a > > > > > race) which finds he client in a state of not having comp_data as the > > > > > E_FREE in _e_comp_x_hook_client_del() frees it and sets it to NULL. > > > > > move the comp_data free to the actual client free (which is the last > > > > > time a client is valid at all) solves this. > > > > > --- > > > > > src/bin/e_client.c | 1 + > > > > > src/bin/e_comp_x.c | 1 - > > > > > 2 files changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > I guess I'll have to take a look at this. Freeing comp_data in > > > > _e_client_free () is wrong in all cases since it's compositor-specific > > > > and guarantees that data will leak. > > > > > > eh? how does it guarantee it will leak? a client free frees the comp_data > > > - > > > you'd otherwise have to never free a client to never free the comp data > > > set > > > ON the client itself. so yes - it's a leak if clients are never freed... > > > whic is a leak in and of itself. > > > > > > simply - it's not a leak now. sure - it's not freed int he same domain > > > where > > > it's allocated - that is true, but the problem is that if that is left > > > there it is ACCESSED in the same domain after being freed - the client is > > > still attached to an object, but the comp data has been freed for the > > > client. so the logical options are to enusre the objects are deleted at > > > the > > > point the comp data is freed OR defer the comp data free until the client > > > is actually freed/cleaned up. i chose option 2 since it had no leaks and > > > was the least amount of changes. > > > > > > > I think you are misunderstanding what I'm saying. The comp_data struct > > itself > > won't leak, but everything in it will leak since this is opaque. Freeing it > > there is a poor workaround to fixing the underlying bug. > > look at the patch... everything inside of it is free/deleted in the same spot. > just the struct ITSELF it deferred to be freed later. so we keep an empty > "comp_data" struct. yeah - i should have nul/0'd out comp_data->damage as that > is freed but still left as a valid resource ref. all of the rest of the > deletion still happens in _e_comp_x_hook_client_del() EXCEPT the free of the > comp_data struct itself. THAT is the only thing not freed there - and JUST > that > is deferred until client free so the comp_data ptr stays valid for as long as > it > is assumed to exist/be valid (i read all the code accessing it - all the code > ASSUMES that if you have a client struct, comp_data is non-null and valid at > all times). Right, I understand what you're saying and what your patch does. It functions as expected now, in X, and I'm not disputing that. Given that comp_data is opaque, however, it's possible (and I have considered) using other types of objects for this. If you were to call free() on something like an Eina_Hash, you would leak data. > > > I just hit this crash myself, and I'm testing a fix for it currently. So far > > it seems to be functioning well, so I'll likely have it up soon. > > ------------------------------------------------------------------------------ WatchGuard Dimension instantly turns raw network data into actionable security intelligence. It gives you real-time visual feedback on key security issues and trends. Skip the complicated setup - simply import a virtual appliance and go from zero to informed in seconds. http://pubads.g.doubleclick.net/gampad/clk?id=123612991&iu=/4140/ostg.clktrk _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
