[Ipopt] Jipopt Java interface

Rafael de Pelegrini Soares rafael at vrtech.com.br
Wed Apr 23 14:18:55 EDT 2008


Hi all!

Carl/Stefan, thank you for the clarification of some details I've
missed. Indeed there is a memory problem with the current Java interface
on trunk.

In order to fix the problem I had to change the code logic a little and
now I think it is working properly. It will be in trunk code soon. Note
that only the C++ side was modified, the Java side (user code) is
unaffected.

Finally, thank you Edson for pointing out the problem.

Cheers.

On Wed, 2008-04-23 at 12:00 -0400, ipopt-request at list.coin-or.org wrote:
> Send Ipopt mailing list submissions to
> 	ipopt at list.coin-or.org
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	http://list.coin-or.org/mailman/listinfo/ipopt
> or, via email, send a message with subject or body 'help' to
> 	ipopt-request at list.coin-or.org
> 
> You can reach the person managing the list at
> 	ipopt-owner at list.coin-or.org
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Ipopt digest..."
> 
> 
> Today's Topics:
> 
>    1. Re: Jipopt Java interface (Edson Valle)
>    2. Re: Jipopt Java interface (Carl Laird)
>    3. Re: Jipopt Java interface (Rafael de Pelegrini Soares)
>    4. Re: Jipopt Java interface (Stefan Vigerske)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Sat, 19 Apr 2008 09:28:51 -0300
> From: Edson Valle <edsoncv at enq.ufrgs.br>
> Subject: Re: [Ipopt] Jipopt Java interface
> To: Rafael de Pelegrini Soares <rafael at vrtech.com.br>
> Cc: coin-ipopt at list.coin-or.org
> Message-ID: <20080419092851.4zruj9xhsa2skks8 at webmail.ufrgs.br>
> Content-Type: text/plain;	charset=ISO-8859-1;	DelSp="Yes";
> 	format="flowed"
> 
>            Dear all
>     The pointer must be pointed to NULL before the deletion (JVM/JNI things).
> 
> JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> (JNIEnv *env,
> jobject obj_this,
> jlong pipopt){
>     // cast back our class
>       Jipopt *problem = (Jipopt *)pipopt;
> 
>       if(problem!=NULL){
>          problem = NULL;
>          delete problem;
>     }
>   }
>               that's it
> 
>                                                         Edson Valle
>                                                     edsoncv at enq.ufrgs.br
> 
> 
> Citando Rafael de Pelegrini Soares <rafael at vrtech.com.br>:
> 
> > Dear Edson,
> >
> > I don't think this is the source of the problem you've experienced,
> > because the Jipopt object is store in a raw pointer. No smart pointer is
> > used there (around line 600 of Jipopt.cpp):
> >
> > /* create the IpoptProblem */
> > Jipopt* problem=new Jipopt(env, obj_this, n, m, nele_jac, nele_hess,
> > index_style);
> >
> >
> > Once no smart pointer is used to store the object a regular 'delete'
> > should be used to destroy it.
> >
> > Hope this helps.
> > Regards.
> >
> >
> > On Fri, 2008-04-18 at 14:06 -0300, Edson Cordeiro do Valle wrote:
> >> Hello all
> >>         I took a look at the Java Native Interface and I found the
> >> following function:
> >>
> >> JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> >> (JNIEnv *env,
> >> jobject obj_this,
> >> jlong pipopt){
> >>     // cast back our class
> >>     Jipopt *problem = (Jipopt *)pipopt;
> >>
> >>     if(problem!=NULL){
> >>         delete problem;
> >>     }
> >> }
> >>
> >> I don't think it is necessary since the smartpointers automatically
> >> cleans the unused references as stated in the documentation:
> >>
> >> " As the SmartPtrs go out of scope, the reference count
> >>  will be decremented and the objects will automatically
> >>  be deleted."
> >>
> >> The calling to this function from java leads the Java Virtual Machine to
> >> crash, so I suggest this function removal.
> >>
> >> Regards
> >>
> >
> >
> 
> 
> 
> 
> 
> ------------------------------
> 
> Message: 2
> Date: Tue, 22 Apr 2008 14:08:43 -0500
> From: "Carl Laird" <carl.d.laird at gmail.com>
> Subject: Re: [Ipopt] Jipopt Java interface
> To: "Edson Valle" <edsoncv at enq.ufrgs.br>
> Cc: coin-ipopt at list.coin-or.org,	Rafael de Pelegrini Soares
> 	<rafael at vrtech.com.br>
> Message-ID:
> 	<b36626570804221208td85b2e6ne474fe5ab45fb2b8 at mail.gmail.com>
> Content-Type: text/plain; charset=ISO-8859-1
> 
> Hello all,
> 
> I must point out that I do not have the Jipopt code so, at the risk of
> saying something completely incorrect, let me add my two bits.
> 
> Ipopt uses SmartPointers internally to reference objects passed to the
> optimize routine. Therefore, even if you create an object and store it
> in a regular pointer, if you give that "raw" pointer to Ipopt AND
> Ipopt puts the pointer into a SmartPointer, it will be deleted when
> its internal reference count hits zero (without you knowing). Since I
> do not have the Jipopt code, I cannot say for sure that this is what
> happens, however it is consistent with a crash on delete. (The last
> SmartPointer deletes the object when it goes out of scope, and then
> the code tries to delete the object again). If someone wants to point
> me to the Jipopt code, I will be happy to have a look at it in more
> detail. (Sorry I have not been keeping up on this interface
> development.)
> 
> On a second note, someone can correct me if I am wrong, but I believe
> that it is perfectly legal to call delete on a NULL pointer in C++.
> The expected behavior is for nothing to happen. Therefore, I think the
> lines below
> >          problem = NULL;
> >          delete problem;
> do not do anything except set the pointer to NULL - I do not think
> that any object is deleted here at all.
> Therefore, if Ipopt is not deleting the problem object internally via
> SmartPointers, then the code above should lead to a memory leak. If
> Ipopt IS deleting the problem object internally via SmartPointers,
> then the code above should not be necessary.
> 
> I hope this helps or at least (since I am ignorant of the Jipopt code)
> does not make matters worse :)
> 
> Cheers,
> 
> Carl.
> 
> On Sat, Apr 19, 2008 at 7:28 AM, Edson Valle <edsoncv at enq.ufrgs.br> wrote:
> >            Dear all
> >     The pointer must be pointed to NULL before the deletion (JVM/JNI things).
> >
> >
> >  JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> >  (JNIEnv *env,
> >  jobject obj_this,
> >  jlong pipopt){
> >     // cast back our class
> >       Jipopt *problem = (Jipopt *)pipopt;
> >
> >       if(problem!=NULL){
> >          problem = NULL;
> >          delete problem;
> >     }
> >   }
> >               that's it
> >
> >                                                         Edson Valle
> >                                                     edsoncv at enq.ufrgs.br
> >
> >
> >  Citando Rafael de Pelegrini Soares <rafael at vrtech.com.br>:
> >
> >
> >
> >  > Dear Edson,
> >  >
> >  > I don't think this is the source of the problem you've experienced,
> >  > because the Jipopt object is store in a raw pointer. No smart pointer is
> >  > used there (around line 600 of Jipopt.cpp):
> >  >
> >  > /* create the IpoptProblem */
> >  > Jipopt* problem=new Jipopt(env, obj_this, n, m, nele_jac, nele_hess,
> >  > index_style);
> >  >
> >  >
> >  > Once no smart pointer is used to store the object a regular 'delete'
> >  > should be used to destroy it.
> >  >
> >  > Hope this helps.
> >  > Regards.
> >  >
> >  >
> >  > On Fri, 2008-04-18 at 14:06 -0300, Edson Cordeiro do Valle wrote:
> >  >> Hello all
> >  >>         I took a look at the Java Native Interface and I found the
> >  >> following function:
> >  >>
> >  >> JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> >  >> (JNIEnv *env,
> >  >> jobject obj_this,
> >  >> jlong pipopt){
> >  >>     // cast back our class
> >  >>     Jipopt *problem = (Jipopt *)pipopt;
> >  >>
> >  >>     if(problem!=NULL){
> >  >>         delete problem;
> >  >>     }
> >  >> }
> >  >>
> >  >> I don't think it is necessary since the smartpointers automatically
> >  >> cleans the unused references as stated in the documentation:
> >  >>
> >  >> " As the SmartPtrs go out of scope, the reference count
> >  >>  will be decremented and the objects will automatically
> >  >>  be deleted."
> >  >>
> >  >> The calling to this function from java leads the Java Virtual Machine to
> >  >> crash, so I suggest this function removal.
> >  >>
> >  >> Regards
> >  >>
> >  >
> >  >
> >
> >
> >
> >  _______________________________________________
> >  Ipopt mailing list
> >  Ipopt at list.coin-or.org
> >  http://list.coin-or.org/mailman/listinfo/ipopt
> >
> 
> 
> 
> -- 
> 
> Carl D. Laird
> Assistant Professor,
> Artie McFerrin Department of Chemical Engineering,
> Texas A&M University
> Ph: (979) 458-4514
> Email: carl.laird at tamu.edu
> 
> 
> ------------------------------
> 
> Message: 3
> Date: Tue, 22 Apr 2008 23:23:33 -0300
> From: Rafael de Pelegrini Soares <rafael at vrtech.com.br>
> Subject: Re: [Ipopt] Jipopt Java interface
> To: Carl Laird <carl.d.laird at gmail.com>
> Cc: Edson Valle <edsoncv at enq.ufrgs.br>, coin-ipopt at list.coin-or.org
> Message-ID: <1208917413.6044.29.camel at rav>
> Content-Type: text/plain
> 
> Dear Carl/Edson/all,
> 
> The Java interface was originally written based on the C interface, at
> that time SmartPtr's could not be used.
> 
> When the code was adapted to use the C++ interface (TNLP class) the
> no-SmartPrt nature was inherited. Of course the SmartPtr way could be used
> but it would be a little tricky (if not useless) because when calling C/C++ from Java the scope goes
> in and out in every function call (objective function, residuals, gradients, etc).
> 
> The point is, do the Ipopt infrastructure stores TNLP's instances internally in SmartPtrs?
> I guess not (this can be seen in example file hs071_main.cpp), hence the current Java
> interface implementation is fine.
> 
> ASAP I'll check out the latest version from repository and will run it under valgring just
> to be sure about any memory invasions/leaks.
> 
> Regards.
> 
> On Tue, 2008-04-22 at 14:08 -0500, Carl Laird wrote:
> > Hello all,
> > 
> > I must point out that I do not have the Jipopt code so, at the risk of
> > saying something completely incorrect, let me add my two bits.
> > 
> > Ipopt uses SmartPointers internally to reference objects passed to the
> > optimize routine. Therefore, even if you create an object and store it
> > in a regular pointer, if you give that "raw" pointer to Ipopt AND
> > Ipopt puts the pointer into a SmartPointer, it will be deleted when
> > its internal reference count hits zero (without you knowing). Since I
> > do not have the Jipopt code, I cannot say for sure that this is what
> > happens, however it is consistent with a crash on delete. (The last
> > SmartPointer deletes the object when it goes out of scope, and then
> > the code tries to delete the object again). If someone wants to point
> > me to the Jipopt code, I will be happy to have a look at it in more
> > detail. (Sorry I have not been keeping up on this interface
> > development.)
> > 
> > On a second note, someone can correct me if I am wrong, but I believe
> > that it is perfectly legal to call delete on a NULL pointer in C++.
> > The expected behavior is for nothing to happen. Therefore, I think the
> > lines below
> > >          problem = NULL;
> > >          delete problem;
> > do not do anything except set the pointer to NULL - I do not think
> > that any object is deleted here at all.
> > Therefore, if Ipopt is not deleting the problem object internally via
> > SmartPointers, then the code above should lead to a memory leak. If
> > Ipopt IS deleting the problem object internally via SmartPointers,
> > then the code above should not be necessary.
> > 
> > I hope this helps or at least (since I am ignorant of the Jipopt code)
> > does not make matters worse :)
> > 
> > Cheers,
> > 
> > Carl.
> > 
> > On Sat, Apr 19, 2008 at 7:28 AM, Edson Valle <edsoncv at enq.ufrgs.br> wrote:
> > >            Dear all
> > >     The pointer must be pointed to NULL before the deletion (JVM/JNI things).
> > >
> > >
> > >  JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> > >  (JNIEnv *env,
> > >  jobject obj_this,
> > >  jlong pipopt){
> > >     // cast back our class
> > >       Jipopt *problem = (Jipopt *)pipopt;
> > >
> > >       if(problem!=NULL){
> > >          problem = NULL;
> > >          delete problem;
> > >     }
> > >   }
> > >               that's it
> > >
> > >                                                         Edson Valle
> > >                                                     edsoncv at enq.ufrgs.br
> > >
> > >
> > >  Citando Rafael de Pelegrini Soares <rafael at vrtech.com.br>:
> > >
> > >
> > >
> > >  > Dear Edson,
> > >  >
> > >  > I don't think this is the source of the problem you've experienced,
> > >  > because the Jipopt object is store in a raw pointer. No smart pointer is
> > >  > used there (around line 600 of Jipopt.cpp):
> > >  >
> > >  > /* create the IpoptProblem */
> > >  > Jipopt* problem=new Jipopt(env, obj_this, n, m, nele_jac, nele_hess,
> > >  > index_style);
> > >  >
> > >  >
> > >  > Once no smart pointer is used to store the object a regular 'delete'
> > >  > should be used to destroy it.
> > >  >
> > >  > Hope this helps.
> > >  > Regards.
> > >  >
> > >  >
> > >  > On Fri, 2008-04-18 at 14:06 -0300, Edson Cordeiro do Valle wrote:
> > >  >> Hello all
> > >  >>         I took a look at the Java Native Interface and I found the
> > >  >> following function:
> > >  >>
> > >  >> JNIEXPORT void JNICALL Java_org_coinor_Ipopt_FreeIpoptProblem
> > >  >> (JNIEnv *env,
> > >  >> jobject obj_this,
> > >  >> jlong pipopt){
> > >  >>     // cast back our class
> > >  >>     Jipopt *problem = (Jipopt *)pipopt;
> > >  >>
> > >  >>     if(problem!=NULL){
> > >  >>         delete problem;
> > >  >>     }
> > >  >> }
> > >  >>
> > >  >> I don't think it is necessary since the smartpointers automatically
> > >  >> cleans the unused references as stated in the documentation:
> > >  >>
> > >  >> " As the SmartPtrs go out of scope, the reference count
> > >  >>  will be decremented and the objects will automatically
> > >  >>  be deleted."
> > >  >>
> > >  >> The calling to this function from java leads the Java Virtual Machine to
> > >  >> crash, so I suggest this function removal.
> > >  >>
> > >  >> Regards
> > >  >>
> > >  >
> > >  >
> > >
> > >
> > >
> > >  _______________________________________________
> > >  Ipopt mailing list
> > >  Ipopt at list.coin-or.org
> > >  http://list.coin-or.org/mailman/listinfo/ipopt
> > >
> > 
> > 
> > 
> > -- 
> > 
> > Carl D. Laird
> > Assistant Professor,
> > Artie McFerrin Department of Chemical Engineering,
> > Texas A&M University
> > Ph: (979) 458-4514
> > Email: carl.laird at tamu.edu
> 
> 
> 
> ------------------------------
> 
> Message: 4
> Date: Wed, 23 Apr 2008 10:35:39 +0200
> From: Stefan Vigerske <stefan at math.hu-berlin.de>
> Subject: Re: [Ipopt] Jipopt Java interface
> To: coin-ipopt at list.coin-or.org
> Message-ID: <480EF4DB.7010006 at math.hu-berlin.de>
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
> 
> Hi,
> 
> > When the code was adapted to use the C++ interface (TNLP class) the
> > no-SmartPrt nature was inherited. Of course the SmartPtr way could be used
> > but it would be a little tricky (if not useless) because when calling C/C++ from Java the scope goes
> > in and out in every function call (objective function, residuals, gradients, etc).
> > 
> > The point is, do the Ipopt infrastructure stores TNLP's instances internally in SmartPtrs?
> 
> Sure, just have a look at the signature of the 
> IpoptApplication::OptimizeTNLP function that you use.
> 
> > I guess not (this can be seen in example file hs071_main.cpp), hence the current Java
> > interface implementation is fine.
> 
> I see there:
> 
> SmartPtr<TNLP> mynlp = new HS071_NLP();
> ...
> status = app->OptimizeTNLP(mynlp);
> 
> Looks like using SmartPtr for me...
> 
> Best,
> Stefan
> 



More information about the Ipopt mailing list