B4Alpha-Aft3r0mega / javacpp

Automatically exported from code.google.com/p/javacpp
GNU General Public License v2.0
0 stars 0 forks source link

Deferred access to @ByVal Pointer callback argument fails #16

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create a callback function with a @ByVal Pointer-derived argument. 
2. Have the callback method store the argument for later use.
3. Call a method on the stored argument after the callback has exited.

What is the expected output? What do you see instead?

Successful method call.

What is actually happening is that the program fails due to invalid memory 
access.

This is caused as the Pointer.address member is initialised with the address of 
the stack-allocated parameter to the _allocate_callback method. This address 
becomes invalid when the callback method exits.

What version of the product are you using? On what operating system?
20120512 on Android.

Please provide any additional information below.

I think that the argument should remain valid until garbage collected by Java 
as in the Java memory-management paradigm all that is required to keep an 
object alive is to maintain any reference to it.

To achieve this the Pointer.address needs to refer to a heap-allocated copy of 
the argument with a deallocator created the same way as return Pointer objects 
are handled.

This behaviour is more consistent with the usual meaning of by-value parameter 
semantics i.e. pass a copy.

Patch attached.

Original issue reported on code.google.com by richardc...@gmail.com on 23 May 2012 at 3:32

Attachments:

GoogleCodeExporter commented 8 years ago
Thinking about it some more, if the designer of the callback function wanted 
efficiency in the first place, they would have used a pointer or const& to pass 
by reference instead, so I guess it makes sense for JavaCPP to make callback 
arguments by value behave like return by value in the case of function call, 
for consistency.

So, I updated the Generator in a similar fashion to your patch, plus a couple 
of other changes to support Cast and FunctionPointer as well. It works with the 
test cast you provided, thanks for that, so I think it's good to ship, but let 
me know if anything needs additional fixing, thanks!

Original comment by samuel.a...@gmail.com on 26 May 2012 at 4:00

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks. Will test on Monday.

I have to say that I'm hugely impressed with the approach your library takes in 
actually attempting to stay object oriented through the java c++ boundary, 
everything else I looked at requires flattening to a c style interface. 

As an aside I think there is a fair bit of scope for factor common code out in 
the generator. It might be worth cresting a class for generating argument 
handling code, creating a instance for each argument and invoking a before and 
after method on each instance in turn. Also a symbol table class that can be 
used to generate unique variable names.

If you're interested in these ideas I could elaborate more. 

Original comment by richardc...@gmail.com on 26 May 2012 at 7:57

GoogleCodeExporter commented 8 years ago
Ok, I've included the changes in the latest release! I'll mark this as fixed, 
but do let me know if for some reason it doesn't work, thank you.

Yes, I was amazed myself that nobody had thought of a similar approach before, 
not even Microsoft, who well settled with C++/CLI.

Anyway, Generator could use some refactoring that's for sure. C++ isn't exactly 
a very consistent language, so I just made up stuff as I needed it, but I also 
did it to prove that a tool like this could actually be done. But I'm afraid we 
need more than a simple refactoring. We need a better design to deal with all 
the corner cases. 

I am hoping that someone at Oracle/Sun or something takes interest in this at 
some point to include something similar in Java SE, but who knows how long that 
will take... 

In any case, of course, please do let me know of your ideas! At the very least, 
if we don't do it ourselves, someone else reading this page could show an 
interest in pursuing them. Thanks!

Original comment by samuel.a...@gmail.com on 27 May 2012 at 12:46

GoogleCodeExporter commented 8 years ago
Samuel,

I have verified that version 0.1 fixes the problem.

thanks

Richard

Original comment by richardc...@gmail.com on 28 May 2012 at 1:24

GoogleCodeExporter commented 8 years ago
Great! BTW, do post your ideas and what not on the mailing list, or private 
e-mail works fine too if that works for you. I would love to hear what you have 
in mind, thanks! 

Original comment by samuel.a...@gmail.com on 28 May 2012 at 1:20