Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 316252

Summary: [Mozilla][Browser]Wrong usage of C.free()
Product: [Eclipse Project] Platform Reporter: Zhou Xing <xzxing>
Component: SWTAssignee: Grant Gayed <grant_gayed>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: matthew.painter, mukund, raji, Silenio_Quarti
Version: 4.0Flags: Silenio_Quarti: review+
Target Milestone: 3.6.1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Sample project to reproduce the JVM crash
none
JVM Crash stack info log file
none
patch
none
patch #2
none
patch #3 none

Description Zhou Xing CLA 2010-06-09 04:09:25 EDT
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
Comment 1 Zhou Xing CLA 2010-06-10 04:07:53 EDT
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.
Comment 2 Zhou Xing CLA 2010-06-10 04:08:56 EDT
Created attachment 171613 [details]
JVM Crash stack info log file
Comment 3 Grant Gayed CLA 2010-06-14 12:34:32 EDT
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();
}
Comment 4 Grant Gayed CLA 2010-07-12 15:29:17 EDT
Created attachment 174090 [details]
patch #2

This revised patch does not assume that the Browser has a LocationProvider available to give the GRE location.
Comment 5 Grant Gayed CLA 2010-07-27 12:29:06 EDT
Created attachment 175336 [details]
patch #3

This patch incorporates suggestions from SSQ's review.
Comment 6 Silenio Quarti CLA 2010-07-27 18:08:50 EDT
+1 for patch #3
Comment 7 Grant Gayed CLA 2010-07-28 09:34:41 EDT
fixed in the 3.6.1 and 3.7 streams > 2100728
Comment 8 Grant Gayed CLA 2011-06-29 11:31:17 EDT
*** Bug 349709 has been marked as a duplicate of this bug. ***