Bugzilla – Full Text Bug Listing
| Summary: | Need to add some try/exception in some SWT JNI call | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | wuzhe <wuzhe> | ||||||||||||||
| Component: | SWT | Assignee: | Silenio Quarti <Silenio_Quarti> | ||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||||
| Severity: | normal | ||||||||||||||||
| Priority: | P3 | CC: | eclipse.felipe, kleind, liujuny, mukund, pwebster, remy.suen, Silenio_Quarti, wuzhe | ||||||||||||||
| Version: | 3.4.2 | ||||||||||||||||
| Target Milestone: | 3.7 M7 | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | Windows XP | ||||||||||||||||
| See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=558501 | ||||||||||||||||
| Whiteboard: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
wuzhe
Created attachment 182458 [details]
add try/exception for two JNI call
That c code you changed is auto-generated, in order to release your patch in the product we would at least have to move it to a custom c code file. That said, using __try __catch in C and throwing a java exception seems wrong to me (in 10 years we never had to do something like that). Also, I am afraid your patch can change the behaviour and break other places that where using regular error code handling and catching other types of exception (like SWTException), this patch would break this other clients because no one expects to get a java.lang.UnknownError from SWT. Passing over to Silenio. Created attachment 182588 [details]
JNI sample (To run as SWT application in Eclipse)
Created attachment 182589 [details]
Sample code snipet to be added into OS.java
Created attachment 182590 [details]
JNI sample DLL
I just attach a sample to demo the problem and solution. With this sample, we saw that, when an GPF exception occur inside JNI a) if we don't catch it, JVM will surely crash b) if we catch and eat it siliently, JVM will not crash. This is not bad. c) if we catch it and throw a Java exception (e.g. java.lang.UnknownError), JVM will not crash, and Java code will be aware of the native exception. Steps to use the sample are, 1. Launch Eclipse IDE 2. Import org.eclipse.swt and org.eclipse.swt.win32.win32.x86 as source project 3. Unzip JNIsampleDLL.zip and copy JNIsample-win32.dll to %workspace%\org.eclipse.swt.win32.win32.x86\. (JNIsample-win32.dll can be rebult from source code JNIsample1.zip if needed.) 3. Open OS.java, and add content from file OS.added.java. 5. Create a SWT project with JniSample1.java 6. Launch JniSample1 as SWT application, and click buttone #1, #2, #3 7. Watch the IDE console The problem is that you are using try-catch in native code to fix the problem. For the real fix you need to understand _why_ exception happens in the native code. For example, is the code calling OleSetMenuDescriptor() with a bad paremeter, in what situation does it happen, can this situation be detected and the call to OleSetMenuDescriptor() avoided ? What information do you have about the crash ? We saw 2 crashes. One is in JNI call to OleSetMenuDescriptor(), another is JNI call to IUnknow.release(), i.e. COM.VtblCall(2, address). Both calls happen in disposing an IE WebBrowser control instances. With a number of months, we saw NO difference in calling sequence and parameters between the success and failure. They are Microsoft code. It make no sense to ask Microsoft to investigate and fix. Because we can not reproduce the crash at will, the crash frequency is around 1 out of 3000 attempts. People are OK for Microsoft IE to crash now and then. For example, if you use IE for 30 days, you may get one crash out of 100*30 opening and closing. You may not complain Microsoft. So far, in 10 years, we can not expect Microsoft to improve their code and fix that crash. However, IBM customer surely complain IBM for this kind of crash. Given a call center with 500 seats, where the agents will open and close the browser control with much higher frequency, we will get more than 500 crash per months. One crash mean one failure in call services. The customer will take this as strong reason to challenge IBM product quality. In my understanding, an exception from JNI will surly crash the JVM. I see no drawback in catching the exception before return to Java code. Do you have any sample to demo that JVM could be still safe at such situation? Or sample on handling the exception in 'regular' way? Felipe and Silenio, any update on this? Created attachment 191466 [details]
patch
This patch adds the try/catch block to those natives taking into consideration that the C code is auto-generated. It also:
- throws SWTError instead of UnknownError
- adds the native exception code to the java exception message
- changes the __except expression from "1,1" to EXCEPTION_EXECUTE_HANDLER. "1,1" did not make much sense to me reading the MSDN docs.
Patch applies to both 3.6.x and 3.7 branches.
I released the above patch to HEAD (>20110317). Note that we strongly believe that this is not the right approach to handle this problem. The segmentation fault should be tracked down and an appropriate fix should be provided by the appropriate part (either SWT or third part code). I understand that given the circumstances, this is not a trivial task. I consider this a temporary work around to avoid the hard crash. You should still try to isolate the problem with your customer and provide us more information so that we can investigate it further. Created attachment 191474 [details]
patch
Previous patch did not compile on WIN64.
|