We need to consider the pass by-value vs pass by-reference semantics,
including "deep copying" the DataObject tree. I have thought about his and
we should discuss this after M2. I'll put together a proposal on another
thread.
For now though I would change your patch and have Operation allocate and
free DataObjectPtrs as appropriate. The memory leak of the odd int, double,
or even a string is not going to be significant but with DataObjectPtr the
DOs will never get freed... or the DataFactory that created them ...etc..
void Operation::addParameter(const DataObjectPtr *parm)
{
LOGINFO(4, "Operation::addParameter(DataObjectPtr)");
parameters.insert(parameters.end(), Parameter((void*)parm,
DATAOBJECT));
}
would become
void Operation::addParameter(const DataObjectPtr *parm)
{
LOGINFO(4, "Operation::addParameter(DataObjectPtr)");
parameters.insert(parameters.end(), Parameter((void*)new
DataObjectPtr(*parm), DATAOBJECT));
}
and the Operation destructor would need to find all the DATAOBJECT type
paramters and delete the DataObjectPtr.
Cheers,
On 10/10/06, Jean-Sebastien Delfino <[EMAIL PROTECTED]> wrote:
Pete Robbins wrote:
> On 10/10/06, Jean-Sebastien Delfino <[EMAIL PROTECTED]> wrote:
>>
>> On the plane on my way to ApacheCon I was playing around with our Web
>> Services support (adding a Web Services client to BigBank) and found a
>> serious problem in Axis2Client and WSServiceProxy, which allocate the
>> DataObject pointers they return on the stack instead of doing a new
>> DataObjectPtr. This causes a memory violation if a client tries to
>> access a DataObject returned from a Web service.
>>
>> I raised JIRA TUSCANY-816 for this, have the fix but need to do a
little
>> more testing before I commit it. If things go well I should be able to
>> commit it later this evening.
>
>
> Having a quick glance at this code I was wondering who frees up the
> memory
> allocated by Axis2Client and set in the Operation?
Currently, nobody :) We had already started to discuss this issue in
this thread:
http://mail-archives.apache.org/mod_mbox/ws-tuscany-dev/200609.mbox/[EMAIL
PROTECTED]
but we don't have a definitive solution for it yet.
> For DataObjectPtr we could have the
> Operation::setParameter/setReturnValue
> methods new up a DataObjectPtr and free it in the destructor. This would
> work nicely with this as it is reference counted. A different solution
> would
> be needed for the other types.
I think you're right, but we need to take a close look at the lifecycle
of the data flowing through the runtime. For example, data to a client
component returned from a service invocation cannot be freed until the
client component finishes.
Here's a first analysis of how we could handle the data set into an
Operation parameters or returnValue. It may not be completely right but
it's a starting point to help trigger some ideas...
Parameters:
- a DataObjectPtr set into the parameter --> can be freed when the
Operation is deleted (in the ServiceProxy::invoke method just before
returning)
- the DataObject pointed to by that DataObjectPtr --> will be freed when
its reference count goes down to 0
- a string* or int* or char** set into the parameter --> can be freed
when the Operation is deleted
- the string or int pointed to by that string* or int* --> can be freed
when the Operation is deleted
- the char* --> document that the component getting the char* is
responsible for freeing it? or free it when the Operation is deleted?
Return value:
- a DataObjectPtr set into the return value --> can be freed when the
Operation is deleted
- the DataObject pointed to by that DataObjectPtr --> will be freed when
its reference count goes down to 0
- a string* or int* or char** set into the parameter --> can be freed
when the Operation is deleted
- the string or int pointed to by that string* or int* --> can be freed
when the Operation is deleted
- the char* --> document that the component getting the char* is
responsible for freeing it? or?
Anyway, I'm not sure what you guys think, but unless we have a very
clear view of how to fix this, I'd suggest to tackle this problem after
the M2 release and live with the small memory leak for now.
Thoughts?
>
> Cheers,
>
>
--
Jean-Sebastien
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
--
Pete
