| Summary: | [Mozilla][Browser]Wrong usage of C.free() | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Zhou Xing <xzxing> | ||||||||||||
| Component: | SWT | Assignee: | Grant Gayed <grant_gayed> | ||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | |||||||||||||
| Severity: | normal | ||||||||||||||
| Priority: | P3 | CC: | matthew.painter, mukund, raji, Silenio_Quarti | ||||||||||||
| Version: | 4.0 | Flags: | Silenio_Quarti:
review+
|
||||||||||||
| Target Milestone: | 3.6.1 | ||||||||||||||
| Hardware: | PC | ||||||||||||||
| OS: | Windows XP | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
Created attachment 171612 [details]
Sample project to reproduce the JVM crash
This is a sample plug-in project to reproduce the JVM crash.
1. import the plug-in project
2. may change the "xulRuntimeBundleNameWin32" or "xulRuntimeBundleNameGtk" or "xulRuntimeBundleNameMacPPC" or "xulRuntimeBundleNameMacX86" parameter's value in "XULRunnerInitializer.java" according to your testing environment.
3. "XULRunnerInitializer.isInstalledXulrunner()" is make sure the XULRunner Environment is correctly setup, and set the "org.eclipse.swt.browser.XULRunnerPath" with the correctly value;
4. run the plug-in project with "Eclipse Application", and Click the "Sample Meny" -> "Sample Action" to run the testing code
5. the JVM will soon crash due to the problem of wrong usage of "C.free"..
6. the crash stack info is in another attachment.
Created attachment 171613 [details]
JVM Crash stack info log file
Created attachment 171847 [details]
patch
test snippet:
public static void main(String[] args) {
Device.DEBUG = true;
//System.setProperty("org.eclipse.swt.browser.XULRunnerPath","e:\\xulrunner-1.9.2\\xulrunner");
Display display = new Display();
Shell shell = new Shell(display);
new Browser(shell, SWT.MOZILLA);
shell.open();
final String cookieName = "cookie_1";
final String TestPage = "file://test.html";
Browser.setCookie(cookieName + "=cookie value", TestPage);
for(int i = 0; i< 10000; i++) {
System.out.println("i: " + i + " " + Browser.getCookie(cookieName, TestPage));
}
display.dispose();
}
Created attachment 174090 [details]
patch #2
This revised patch does not assume that the Browser has a LocationProvider available to give the GRE location.
Created attachment 175336 [details]
patch #3
This patch incorporates suggestions from SSQ's review.
+1 for patch #3 fixed in the 3.6.1 and 3.7 streams > 2100728 *** Bug 349709 has been marked as a duplicate of this bug. *** |
Build Identifier: M20090211-1700 In Eclipse SWT Mozilla Browser, Runnable "MozillaGetCookie" in Mozilla.java, uses "C.free()" method to release the "cookieString" which point to the memory space allocate by the XPCOM ToNewString()( reference to the source code of GetCookieString() ). See snippet: MozillaGetCookie = new Runnable() { public void run() { ... C.free (cookieString); ... } }; Mozilla Developer Center : "https://developer.mozilla.org/En/Mozilla_internal_string_guide#Callee-allocated_Parameters" said that, char *ToNewCString(nsACString&) return a buffer allocated using XPCOM's allocator instead of the traditional allocator (malloc, etc.). You should use NS_Free() to deallocate the result when you no longer need it. So we should not use "C.free()" in this situation(Leads to crash sometimes) and use "XPCOM.nsfree()" instead. Reproducible: Always