From: Art C. <ac...@vl...> - 2008-10-26 22:27:55
Attachments:
diff.txt
|
Hi, First off, Swig rocks... this is merely a suggestion to make it hopefully rock a little more in Java. Please let me know your thoughts In the Java module of Swig (as of 1.3.37) the string "swigCPtr" is assumed as the name of the variable that holds the native pointer to the C Object, and this string is inserted directly in the Java proxy class when calls are made requiring a "this" pointer. This works great, but swig also auto-generates a delete() method that will set swigCPtr to 0, and auto-generates C++ method calls that essentially do (swigCPtr)->func(). If someone has called delete() in the Java wrapper, and then a class (non static) method, they'll get a core-dump as opposed to a more friendly error message. That said, it would be inappropriate to assume that everyone wants to do something more intelligent (and consequently less efficient) that always assume swigCPtr is valid, so I propose the following change: Allow people to override the string used to find the current native pointer from the JNI proxy class, but default to "swigCPtr". For example: %pragma(java) jniclassthisptrname="mySafeGetCPtr()". That way if no one overrides this "jniclassthisptrname" setting they'll get today's efficient (but potentially unsafe) "swigCPtr". If they do override it, they can intercept deferences of swigCPtr before it causes the core-dump in native code and instead raise an exception. The attached patch does exactly that for Revision 10894. It should be 100% backwards compatible. The main reason I'd like this feature is, I admit it, to protect me (as the maintainer of a swig-based library) from stupid mistakes by my users. Yes, they should NEVER use an object after they call delete(), and yes, everytime I point that out to them they agree. Still the sight of their JVM core-dumping always brings them to me screaming, and I'd like to instead return a nice Java exception so my users are a little, well, calmer :) Also, if there is a way to do this with Swig without patching swig, please let me know (but I've tried all sorts of typemaps, and keep coming back to the issue that I can't tell from a typemap if I'm in a static or non-static method). Thanks in advance, - Art Clarke p.s. I also added this to the sourceforge tracker: https://sourceforge.net/tracker2/?func=detail&aid=2198889&group_id=1645&atid=301645 |
From: William S F. <ws...@fu...> - 2008-10-27 20:58:05
|
Art Clarke wrote: > Hi, > > First off, Swig rocks... this is merely a suggestion to make it > hopefully rock a little more in Java. Please let me know your > thoughts > > In the Java module of Swig (as of 1.3.37) the string "swigCPtr" is > assumed as the name of the variable that holds the native pointer to > the C Object, and this string is inserted directly in the Java proxy > class when calls are made requiring a "this" pointer. > > This works great, but swig also auto-generates a delete() method that > will set swigCPtr to 0, and auto-generates C++ method calls that > essentially do (swigCPtr)->func(). If someone has called delete() in > the Java wrapper, and then a class (non static) method, they'll get a > core-dump as opposed to a more friendly error message. > > That said, it would be inappropriate to assume that everyone wants to > do something more intelligent (and consequently less efficient) that > always assume swigCPtr is valid, so I propose the following change: > > Allow people to override the string used to find the current native > pointer from the JNI proxy class, but default to "swigCPtr". > > For example: > > %pragma(java) jniclassthisptrname="mySafeGetCPtr()". > > That way if no one overrides this "jniclassthisptrname" setting > they'll get today's efficient (but potentially unsafe) "swigCPtr". If > they do override it, they can intercept deferences of swigCPtr before > it causes the core-dump in native code and instead raise an exception. > > The attached patch does exactly that for Revision 10894. It should be > 100% backwards compatible. > > The main reason I'd like this feature is, I admit it, to protect me > (as the maintainer of a swig-based library) from stupid mistakes by my > users. Yes, they should NEVER use an object after they call delete(), > and yes, everytime I point that out to them they agree. Still the > sight of their JVM core-dumping always brings them to me screaming, > and I'd like to instead return a nice Java exception so my users are a > little, well, calmer :) Also, if there is a way to do this with Swig > without patching swig, please let me know (but I've tried all sorts of > typemaps, and keep coming back to the issue that I can't tell from a > typemap if I'm in a static or non-static method). > This is the bit of magic you are looking for: %typemap(check) SWIGTYPE *self %{ if (!$1) { SWIG_JavaThrowException(jenv, SWIG_JavaNullPointerException, "swigCPtr null"); return $null; } %} Explanation: The 'check' typemap is for checking all values arriving in the C/C++ layer. SWIGTYPE is the generic match all type and 'self' is the name of the parameter that the 'this' pointer as marshalled into the C/C++ layer as. The only caveat to be aware of is if you happen to have any parameters that are pointers with the name 'self', the check is also applied to them. Note that this solution will work with all language modules. > Thanks in advance, > > - Art Clarke > > p.s. > I also added this to the sourceforge tracker: > https://sourceforge.net/tracker2/?func=detail&aid=2198889&group_id=1645&atid=301645 > The patch is quite neat, but as there is an alternative and it just adds more complexity, I'd rather the 'check' typemap approach was used. A patch for the documentation containing this use case with solution would be very useful ;) William |
From: Art C. <ac...@vl...> - 2008-10-27 22:33:52
Attachments:
diff.txt
|
That's the right answer; not my crappy change. Attached is a documentation patch that explains that. I'll update the tracker as well. - Art On Mon, Oct 27, 2008 at 1:57 PM, William S Fulton <ws...@fu...> wrote: > Art Clarke wrote: >> >> Hi, >> >> First off, Swig rocks... this is merely a suggestion to make it >> hopefully rock a little more in Java. Please let me know your >> thoughts >> >> In the Java module of Swig (as of 1.3.37) the string "swigCPtr" is >> assumed as the name of the variable that holds the native pointer to >> the C Object, and this string is inserted directly in the Java proxy >> class when calls are made requiring a "this" pointer. >> >> This works great, but swig also auto-generates a delete() method that >> will set swigCPtr to 0, and auto-generates C++ method calls that >> essentially do (swigCPtr)->func(). If someone has called delete() in >> the Java wrapper, and then a class (non static) method, they'll get a >> core-dump as opposed to a more friendly error message. >> >> That said, it would be inappropriate to assume that everyone wants to >> do something more intelligent (and consequently less efficient) that >> always assume swigCPtr is valid, so I propose the following change: >> >> Allow people to override the string used to find the current native >> pointer from the JNI proxy class, but default to "swigCPtr". >> >> For example: >> >> %pragma(java) jniclassthisptrname="mySafeGetCPtr()". >> >> That way if no one overrides this "jniclassthisptrname" setting >> they'll get today's efficient (but potentially unsafe) "swigCPtr". If >> they do override it, they can intercept deferences of swigCPtr before >> it causes the core-dump in native code and instead raise an exception. >> >> The attached patch does exactly that for Revision 10894. It should be >> 100% backwards compatible. >> >> The main reason I'd like this feature is, I admit it, to protect me >> (as the maintainer of a swig-based library) from stupid mistakes by my >> users. Yes, they should NEVER use an object after they call delete(), >> and yes, everytime I point that out to them they agree. Still the >> sight of their JVM core-dumping always brings them to me screaming, >> and I'd like to instead return a nice Java exception so my users are a >> little, well, calmer :) Also, if there is a way to do this with Swig >> without patching swig, please let me know (but I've tried all sorts of >> typemaps, and keep coming back to the issue that I can't tell from a >> typemap if I'm in a static or non-static method). >> > This is the bit of magic you are looking for: > > %typemap(check) SWIGTYPE *self %{ > if (!$1) { > SWIG_JavaThrowException(jenv, SWIG_JavaNullPointerException, "swigCPtr > null"); > return $null; > } > %} > > Explanation: The 'check' typemap is for checking all values arriving in the > C/C++ layer. SWIGTYPE is the generic match all type and 'self' is the name > of the parameter that the 'this' pointer as marshalled into the C/C++ layer > as. The only caveat to be aware of is if you happen to have any parameters > that are pointers with the name 'self', the check is also applied to them. > Note that this solution will work with all language modules. > >> Thanks in advance, >> >> - Art Clarke >> >> p.s. >> I also added this to the sourceforge tracker: >> >> https://sourceforge.net/tracker2/?func=detail&aid=2198889&group_id=1645&atid=301645 >> > > The patch is quite neat, but as there is an alternative and it just adds > more complexity, I'd rather the 'check' typemap approach was used. A patch > for the documentation containing this use case with solution would be very > useful ;) > > William > > |
From: William S F. <ws...@fu...> - 2008-11-16 21:13:20
|
Art Clarke wrote: > That's the right answer; not my crappy change. > > Attached is a documentation patch that explains that. I'll update the > tracker as well. Thanks, committed with some minor mods. William |