http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2453
--- Comment #21 from Balint Reczey <balint.reczey@xxxxxxxxxxxx> 2008-04-30 06:30:17 GMT ---
I think that we cannot resolv the problem withLua's garbage collector, because
the tvb is freed by wireshark.
Our problem
http://www.lua.org/pil/28.5.html
(In reply to comment #18)
> Created an attachment (id=1758)
--> (http://bugs.wireshark.org/bugzilla/attachment.cgi?id=1758) [details]
> remove the "outstanding_stuff" handling completely
>
> I would remove the "outstanding_stuff" completely. I tend to think that
> implementing the __gc method can clear all the data. In pushXxx(L, xxx)
> functions (see epan/wslua/wslua.h) a new userdata is created with the function
> lua_newuserdata(). Then the pointer is actually not checked if it is NULL or
> not, but this is an other story. So we have a pointer and use this pointer to
> fill the content of the buffer.
>
> In the __gc function this area could be filled with zeros, if it is needed at
> all.
>
> What is not clear for me that what should be the pair of the
> luaL_getmetatable(), lua_setmetatable(), and the "push_code" parameter in __gc.
>
(In reply to comment #20)
> (In reply to comment #17)
> > I don't really understand why should be there *anything* which has to be
> > cleared. Lua will manage the memory and call the __gc if something has to be
> > cleared. What can be cleared are the actual userdata and not the pointer itself
> > (see epan/wslua/wslua.h)
> >
> > If I understand the situation correctly maybe the __gc function should be
> > implemented instead of reinventing the wheel with this "outstanding_stuff"
> > thing.
> >
>
> The outstanding stuff is there to avoid leaving "pointers to junk" in the lua
> VM.
>
> if the lua user copies a tvb and keeps it after the current packet that pointer
> will become invalid as soon as the ws core cleans it up before the next packet.
> By NULLifing it we avoid it to crash if used later.
>
> if for example we have __gc freeing the tvb itself there's probably another 30
> places Wireshark could crash when trying to use the freed tvb.
>
> So the whole "outstanding_X thing" is there simply because the lifespan of the
> Lua variable containing the object and the object itself is not related.
>
> I agree that the best solution would be to forcefully purge variables from the
> Lua VM (e.g. turn them into nil) when the underlying object is being freed or
> it has become invalid; But unfortunately I'm not aware of any way to achieve
> this.
>
Unfortunatelly that method does not seem to work.
wslua_tvb.c:
void clear_outstanding_tvbs(void) {
while (outstanding_stuff->len) {
void** p = (void**)g_ptr_array_remove_index_fast(outstanding_stuff,0);
*p = NULL;
^^^^^^^^^^
We could use p there which is Tvb*, but not *p which may
point to a freed memory location, like in the valgrind example.
It can be easily checked using DDD.
}
}
void* push_Tvb(lua_State* L, Tvb tvb) {
void** p = (void**)pushTvb(L,tvb);
^^^^^^^^^^^^^^^^^^^^^^^^
pushTvb returns Tvb* and we cast it to void** which does not
cause any trouble since we return p (and we do not use int anywhere)
g_ptr_array_add(outstanding_stuff,p);
return p;
}
I think we should disable the outstanding stuff for now because it causes
random crashes and warn users not to keep references to buffers/trees/pinfos
from previous packets.
As a final solution, i'll try to fix the problem of junk pointers by using
light userdata.
--
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
