quickfix memory leak

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

quickfix memory leak

naka
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html


Hello.

I found memory leak.

quickfix 1.14.0

Session Factory line 226
  DataDictionary * pCopyOfDD = new DataDictionary(*pDD);

when delete this variable?



------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Oren Miller
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html

Thanks, we are investigating.

> -------- Original Message --------
> Subject: [Quickfix-developers] quickfix memory leak
> From: Man T <[hidden email]>
> Date: Thu, September 11, 2014 9:37 am
> To: [hidden email]
>
>
> QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
> QuickFIX Support: http://www.quickfixengine.org/services.html<hr>Hello.
>
> I found memory leak.
>
> quickfix 1.14.0
>
> Session Factory line 226
>   DataDictionary * pCopyOfDD = new DataDictionary(*pDD);
>
> when delete this variable?<hr>------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk<hr>_______________________________________________
> Quickfix-developers mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quickfix-developers

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Viktor Pogrebnyak
In reply to this post by naka
I don't see where this code can leak except program termination due to missing cleanup for SessionFactoryu::m_dictionaries container.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Oren Miller
In reply to this post by naka
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html

Agreed.  This the relevant destructor at the top of the file:

SessionFactory::~SessionFactory()
{
  Dictionaries::iterator i = m_dictionaries.begin();
  for ( ; i != m_dictionaries.end(); ++i )
    delete i->second;
}

Looks to me like the dictionaries are being destroyed in the same
context they are created.  Seems fine.

> -------- Original Message --------
> Subject: Re: [Quickfix-developers] quickfix memory leak
> From: Viktor Pogrebnyak <[hidden email]>
> Date: Thu, September 11, 2014 12:43 pm
> To: [hidden email]
>
>
> QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
> QuickFIX Support: http://www.quickfixengine.org/services.html
>
> I don't see where this code can leak except program termination due to
> missing cleanup for SessionFactoryu::m_dictionaries container.
>
>
>
> --
> View this message in context: http://quickfix.13857.n7.nabble.com/quickfix-memory-leak-tp6680p6682.html
> Sent from the QuickFIX - Dev mailing list archive at Nabble.com.
>
> ------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
> _______________________________________________
> Quickfix-developers mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quickfix-developers

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Oren Miller
In reply to this post by naka
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html

So it turns out there was a leak here.  The returned copies were not
being stored in that collection and were never disposed of.  I solved
this right now by putting them in std::shared_ptr objects.  They are
supported in VS 2010, which is the oldest VS we currently support.  As
far as I can tell it's supported as far back as gcc 4.3 which I think
has been out for 7-8 years now.  So I think we're probably ok there.
Would this realistically be a problem for anyone?

I think it would be nice to be able to use some modern smart pointers.
When the project was started back in 2001, there was really nothing
standardized that was widely distributed other than the dependency
monster boost.  Now that there is I think it would help us avoid these
types of memory pitfalls.

> -------- Original Message --------
> Subject: Re: [Quickfix-developers] quickfix memory leak
> From: Viktor Pogrebnyak <[hidden email]>
> Date: Thu, September 11, 2014 12:43 pm
> To: [hidden email]
>
>
> QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
> QuickFIX Support: http://www.quickfixengine.org/services.html
>
> I don't see where this code can leak except program termination due to
> missing cleanup for SessionFactoryu::m_dictionaries container.
>
>
>
> --
> View this message in context: http://quickfix.13857.n7.nabble.com/quickfix-memory-leak-tp6680p6682.html
> Sent from the QuickFIX - Dev mailing list archive at Nabble.com.
>
> ------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
> _______________________________________________
> Quickfix-developers mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quickfix-developers

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

K. Frank
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html

Hi Oren!

On Fri, Sep 12, 2014 at 12:49 AM,  <[hidden email]> wrote:

>
> So it turns out there was a leak here.  The returned copies were not
> being stored in that collection and were never disposed of.  I solved
> this right now by putting them in std::shared_ptr objects.
> ...
> I think it would be nice to be able to use some modern smart pointers.
> When the project was started back in 2001, there was really nothing
> standardized that was widely distributed other than the dependency
> monster boost.  Now that there is I think it would help us avoid these
> types of memory pitfalls.

I would personally advocate for the use of std::shared_ptr (and other
standard smart pointers, as appropriate), but, of course, this would
make QuickFIX (partially) dependent on c++11.

For me, that's fine (in fact preferable), but I understand that many
users may have older toolchains that don't support the standard
smart pointers (or other c++11 / c++14 features).  What's been the
QuickFIX philosophy about adopting new language features when
they're beneficial?


Thanks.


K. Frank


>> -------- Original Message --------
>> Subject: Re: [Quickfix-developers] quickfix memory leak
>>
>> I don't see where this code can leak except program termination due to
>> missing cleanup for SessionFactoryu::m_dictionaries container.
>> ...

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Oren Miller
In reply to this post by naka
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
QuickFIX Support: http://www.quickfixengine.org/services.html

We have been pretty conservative up until this point, but that was
largely due to how poor and divergent compilers were when the project
first started.  I'm talking right in the bad old days of gcc "2.96".
Any advanced feature supported by one compiler version, was probably
non-existent in others.  

We are also starting to get more aggressive dropping out of date
toolchains.  We supported VS6 for a long long time, but we now only
supports VS10 and up.

The philosophy on the whole has been to get quickfix up and running for
people with as few dependencies as possible.  That left libraries like
boost out.  I don't want anyone to have to depend on such a large and
complicated dependency to use the library.  Since std::shared_ptr comes
with most compilers nowadays, I'm happy to start using it.  With the
latest 1.14.1 coming out, we were able to drop the dependencies on
libxml2 and msxml.  This made our PyPI installation a lot easier too.

I feel that if anyone is running a legacy system that old, they must
really love legacy software and can continue to use previous stable
releases of QuickFIX.

I'd really like to use more of C++11.  In fact I was looking to convert
all our std for loops to range based loops, but alas VS 2010 doesn't
support them.  They make the code so much cleaner.

> -------- Original Message --------
> Subject: Re: [Quickfix-developers] quickfix memory leak
> From: "K. Frank" <[hidden email]>
> Date: Fri, September 12, 2014 8:44 am
> To: Quickfix Developers List <[hidden email]>
>
>
> QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/index.html
> QuickFIX Support: http://www.quickfixengine.org/services.html
>
> Hi Oren!
>
> On Fri, Sep 12, 2014 at 12:49 AM,  <[hidden email]> wrote:
> >
> > So it turns out there was a leak here.  The returned copies were not
> > being stored in that collection and were never disposed of.  I solved
> > this right now by putting them in std::shared_ptr objects.
> > ...
> > I think it would be nice to be able to use some modern smart pointers.
> > When the project was started back in 2001, there was really nothing
> > standardized that was widely distributed other than the dependency
> > monster boost.  Now that there is I think it would help us avoid these
> > types of memory pitfalls.
>
> I would personally advocate for the use of std::shared_ptr (and other
> standard smart pointers, as appropriate), but, of course, this would
> make QuickFIX (partially) dependent on c++11.
>
> For me, that's fine (in fact preferable), but I understand that many
> users may have older toolchains that don't support the standard
> smart pointers (or other c++11 / c++14 features).  What's been the
> QuickFIX philosophy about adopting new language features when
> they're beneficial?
>
>
> Thanks.
>
>
> K. Frank
>
>
> >> -------- Original Message --------
> >> Subject: Re: [Quickfix-developers] quickfix memory leak
> >>
> >> I don't see where this code can leak except program termination due to
> >> missing cleanup for SessionFactoryu::m_dictionaries container.
> >> ...
>
> ------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
> _______________________________________________
> Quickfix-developers mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quickfix-developers

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

K. Frank
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/

Hi Oren!

Thanks for your reply.

On Fri, Sep 12, 2014 at 11:53 AM,  <[hidden email]> wrote:

> We have been pretty conservative up until this point, but that was
> largely due to how poor and divergent compilers were when the project
> first started.
> ...
> We are also starting to get more aggressive dropping out of date
> toolchains.  We supported VS6 for a long long time, but we now only
> supports VS10 and up.
>
> The philosophy on the whole has been to get quickfix up and running for
> people with as few dependencies as possible.

I like that philosophy.  (But why then xml?  That was the one thing that
caused me the most nuisance "porting" QuickFIX to mingw_w64.)

> That left libraries like boost out.

I wholeheartedly agree.  Boost has some good stuff in it, but it's hugely
bloated.  I think there may be some conceit on boost's part.  (Our code
don't stink ...)  Why should I have to download a template-metaprogramming
state-machine framework just to use a smart pointer?  Boost really ought
to make a (small) base library of low-level utilities that are clearly broadly
useful (e.g. smart pointers).  Then they could implement all of their
whiz-bang experiments in separate add-on libraries.

> ...
> With the
> latest 1.14.1 coming out, we were able to drop the dependencies on
> libxml2 and msxml.  This made our PyPI installation a lot easier too.

As noted above, xml was something of an issue for me.  How did you
get rid of the libxml2 / msxml dependency?

> I feel that if anyone is running a legacy system that old, they must
> really love legacy software and can continue to use previous stable
> releases of QuickFIX.

Personally, I agree with this.  But I understand and respect that
many projects take a more legacy-focused approach.  But I'm with
you guys -- keep moving forward!

> I'd really like to use more of C++11.  In fact I was looking to convert
> all our std for loops to range based loops, but alas VS 2010 doesn't
> support them.  They make the code so much cleaner.

(Then drop VS 2010.  Hee-hee ...)

Another, more substantive change you could make would be to switch
your threading to c++11's std::thread (and friends).  Let the tool vendors
(i.e., VS and gcc) worry about the platform-specific code.  std::thread is
broadly supported now, among others, by VS, gcc on linux, and gcc on
windows (mingw-w64).


Best regards.


K. Frank


>> -------- Original Message --------
>> Subject: Re: [Quickfix-developers] quickfix memory leak
>> From: "K. Frank" <[hidden email]>
>>
>> Hi Oren!
>>
>> On Fri, Sep 12, 2014 at 12:49 AM,  <[hidden email]> wrote:
>> >
>> > So it turns out there was a leak here.  The returned copies were not
>> > being stored in that collection and were never disposed of.  I solved
>> > this right now by putting them in std::shared_ptr objects.
>> > ...
>> > I think it would be nice to be able to use some modern smart pointers.
>> > When the project was started back in 2001, there was really nothing
>> > standardized that was widely distributed other than the dependency
>> > monster boost.  Now that there is I think it would help us avoid these
>> > types of memory pitfalls.
>>
>> I would personally advocate for the use of std::shared_ptr (and other
>> standard smart pointers, as appropriate), but, of course, this would
>> make QuickFIX (partially) dependent on c++11.
>>
>> For me, that's fine (in fact preferable), but I understand that many
>> users may have older toolchains that don't support the standard
>> smart pointers (or other c++11 / c++14 features).  What's been the
>> QuickFIX philosophy about adopting new language features when
>> they're beneficial?
>>
>> Thanks.
>>
>> K. Frank
>> ...

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: quickfix memory leak

Oren Miller
In reply to this post by naka
QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/

Context of the time.  This is back in 2001.  The future of everyone
passing everything around json objects was a long way off.  Did you know
that back then the FIX protocol standard was distributed in Microsoft
Word documents?

As far as I know we were the first to make a consistent well formed
document that defined all the fields and messages, pre-dating the FIX
repository (which for a long time cost money).  This involved saving the
the word documents as html and writing a flex parser to generate the
xml.  If you want to get an idea of how dirty even the html was take a
look at one of the documents and the parser.

https://raw.githubusercontent.com/quickfix/quickfix/v1.3.2/spec/fix-42-with_errata_20010501.html
https://github.com/quickfix/quickfix/blob/v1.3.2/spec/FixLexer.l

We turned that into this!

https://github.com/quickfix/quickfix/blob/v1.3.2/spec/FIX42.xml

So creating a lightweight machine readable version of the spec was a
high priority.  I wanted to make it available for everyone to use, and
xml was established and pretty well suited to the task.  There were a
lot of xml tools available at the time, so people could easily make use
of these documents.  Originally these documents were created for
distribution and to act as our code generators.  It made sense at that
point to also use it the runtime data dictionary.  Those dependencies
always caused me the most trouble also.  MSXML didn't even come with
windows or visual studio back then.

There is now a great little project called pugixml. One source and one
header.  I just embedded it into the project and that was that.  It's
smaller, faster, and easier to use.

http://pugixml.org/

Right now my plan for Visual Studio is to support any version for which
Microsoft has a download link to a free express edition.  Since we went
native for C#, we no longer need to be able to compile two languages in
the same solution.  This removes the dependency of needing a commercial
version of Visual Studio (great for students and enthusiasts), and saves
me the cost of keeping an MSDN subscription.

Unfortunately the VS10 binaries still seem to be quite popular.  Almost
as much as VS12.  VS11 is actually far less popular than either.

> -------- Original Message --------
> Subject: Re: [Quickfix-developers] quickfix memory leak
> From: "K. Frank" <[hidden email]>
> Date: Fri, September 12, 2014 6:17 pm
> To: Quickfix Developers List <[hidden email]>
>
>
> QuickFIX Documentation: http://www.quickfixengine.org/quickfix/doc/html/
>
> Hi Oren!
>
> Thanks for your reply.
>
> On Fri, Sep 12, 2014 at 11:53 AM,  <[hidden email]> wrote:
> > We have been pretty conservative up until this point, but that was
> > largely due to how poor and divergent compilers were when the project
> > first started.
> > ...
> > We are also starting to get more aggressive dropping out of date
> > toolchains.  We supported VS6 for a long long time, but we now only
> > supports VS10 and up.
> >
> > The philosophy on the whole has been to get quickfix up and running for
> > people with as few dependencies as possible.
>
> I like that philosophy.  (But why then xml?  That was the one thing that
> caused me the most nuisance "porting" QuickFIX to mingw_w64.)
>
> > That left libraries like boost out.
>
> I wholeheartedly agree.  Boost has some good stuff in it, but it's hugely
> bloated.  I think there may be some conceit on boost's part.  (Our code
> don't stink ...)  Why should I have to download a template-metaprogramming
> state-machine framework just to use a smart pointer?  Boost really ought
> to make a (small) base library of low-level utilities that are clearly broadly
> useful (e.g. smart pointers).  Then they could implement all of their
> whiz-bang experiments in separate add-on libraries.
>
> > ...
> > With the
> > latest 1.14.1 coming out, we were able to drop the dependencies on
> > libxml2 and msxml.  This made our PyPI installation a lot easier too.
>
> As noted above, xml was something of an issue for me.  How did you
> get rid of the libxml2 / msxml dependency?
>
> > I feel that if anyone is running a legacy system that old, they must
> > really love legacy software and can continue to use previous stable
> > releases of QuickFIX.
>
> Personally, I agree with this.  But I understand and respect that
> many projects take a more legacy-focused approach.  But I'm with
> you guys -- keep moving forward!
>
> > I'd really like to use more of C++11.  In fact I was looking to convert
> > all our std for loops to range based loops, but alas VS 2010 doesn't
> > support them.  They make the code so much cleaner.
>
> (Then drop VS 2010.  Hee-hee ...)
>
> Another, more substantive change you could make would be to switch
> your threading to c++11's std::thread (and friends).  Let the tool vendors
> (i.e., VS and gcc) worry about the platform-specific code.  std::thread is
> broadly supported now, among others, by VS, gcc on linux, and gcc on
> windows (mingw-w64).
>
>
> Best regards.
>
>
> K. Frank
>
>
> >> -------- Original Message --------
> >> Subject: Re: [Quickfix-developers] quickfix memory leak
> >> From: "K. Frank" <[hidden email]>
> >>
> >> Hi Oren!
> >>
> >> On Fri, Sep 12, 2014 at 12:49 AM,  <[hidden email]> wrote:
> >> >
> >> > So it turns out there was a leak here.  The returned copies were not
> >> > being stored in that collection and were never disposed of.  I solved
> >> > this right now by putting them in std::shared_ptr objects.
> >> > ...
> >> > I think it would be nice to be able to use some modern smart pointers.
> >> > When the project was started back in 2001, there was really nothing
> >> > standardized that was widely distributed other than the dependency
> >> > monster boost.  Now that there is I think it would help us avoid these
> >> > types of memory pitfalls.
> >>
> >> I would personally advocate for the use of std::shared_ptr (and other
> >> standard smart pointers, as appropriate), but, of course, this would
> >> make QuickFIX (partially) dependent on c++11.
> >>
> >> For me, that's fine (in fact preferable), but I understand that many
> >> users may have older toolchains that don't support the standard
> >> smart pointers (or other c++11 / c++14 features).  What's been the
> >> QuickFIX philosophy about adopting new language features when
> >> they're beneficial?
> >>
> >> Thanks.
> >>
> >> K. Frank
> >> ...
>
> ------------------------------------------------------------------------------
> Want excitement?
> Manually upgrade your production database.
> When you want reliability, choose Perforce
> Perforce version control. Predictably reliable.
> http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
> _______________________________________________
> Quickfix-developers mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/quickfix-developers

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
Quickfix-developers mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/quickfix-developers
Loading...