logging-log4cxx-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Westman <peter.west...@visualact.se>
Subject Re: Cannot figure out how to clean correctly
Date Thu, 27 Oct 2016 09:11:57 GMT
I think mixing smart pointers and static class allocation would just mix 
things up. We should instead choose when to use one over the other.

We are currently returning new, dynamically allocated level objects for 
each call to getInfo() etc. This memory must be freed. When logging from 
a logger, a call to getInfo() will allocate a new level object in a 
smart pointer, which is returned, used while logging and then when the 
smart pointer is destructed the dynamically allocated object is deleted. 
We used to have static, dynamically created level objects, that had to 
be stored in smart pointers since they were dynamically allocated. Since 
they were created at runtime, it wouldn't be thread safe when two calls 
would happen simultaneously to the same function.

My suggestion is to have getInfo return a const Level& instead of a 
LevelPtr. This would in turn be a reference to a static class member 
(allocated without new) meaning its created at compile time. Since it 
isn't dynamically created, no smart pointer is needed, and it doesnt 
have to be deallocated. This would be more like the initial 
pre-LOG4CXX-394 implementation in that the same object is used for each 
level, but we would just not have a smart pointer. Since the object is 
created at compile time and never modified it would be thread safe, and 
would solve LOG4CXX-322 for levels. Loggers cannot use this pattern, 
since they are not known at compile time.

/Peter



On 10/27/2016 10:45 AM, Thorsten Schöning wrote:
> Guten Tag Peter Westman,
> am Donnerstag, 27. Oktober 2016 um 09:05 schrieben Sie:
>
>> For levels though, I dont really see why we even have smart pointers? We
>> could just as easily create the variables statically in the class,
> That should be thread-safe regarding LOGCXX-322 as well, shouldn't it?
>
> https://issues.apache.org/jira/browse/LOGCXX-322
>
>> and
>> use them as const references to the original levels.
> Do you suggest replacing LevelPtr as the return type of Level::get*?
>
> Because else we seem to need some LevelPtr as static base, because
> ObjectPtr-CTORs seem to only be capable of using raw pointers or
> ObjectPtrs. But if ObjectPtr leaks now, shouldn't it as well if used
> statically in the class? So one would need to find the reason for the
> leak anyway?
>
> Maybe we should enhance ObjectPtr to a mode for back compatibility
> reasons where a managed pointer is not deleted at all? This way the
> interface could stay the same and static levels instead of LevelPtrs
> could be used in the class. A new LevelPtr instance would be created
> on each invocation of Level::get*, though.
>
>> Right now the
>> library is allocating new dynamic memory on a call basis, which seems
>> pretty wasteful to me. If we make the storage static (which it was prior
>> to LOG4CXX-394/r1566655 anyway). In some cases like the Logger class
>> that stores a changeable level it would have to be a plain pointer, but
>> I personally dont see a problem with that.
>
> Mit freundlichen Grüßen,
>
> Thorsten Schöning
>


Mime
View raw message