Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 388996 - [compiler][resource] Incorrect 'potential resource leak'
Summary: [compiler][resource] Incorrect 'potential resource leak'
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 4.3   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.3 M2   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 376938 400523 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-09-06 16:23 EDT by Diego Plentz CLA
Modified: 2019-12-26 16:20 EST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Diego Plentz CLA 2012-09-06 16:23:46 EDT
public void processRequest(RequestContext requestContext, ResponseContext responseContext, ServletContext servletContext) throws IOException {
		OutputStream bao = null;
		CountingOutputStream cos = null;
		OutputStreamWriter osw = null;

		try {
			DefaultSyncParameters params = DefaultSyncParameters.parseParams(requestContext.getParams());
			SyncUser syncUser = new SyncUser(params.getLogin(), params.getPassword(), requestContext.getIpAddress());

			SynchronizationManager manager = SynchronizerUtil.getInstance().getManager();
			manager.getSynchronizationCache().setUser(syncUser);
			manager.authenticateUser(syncUser);

			HttpServletResponse response = responseContext.getResponse();
			response.setContentType(SyncGlobals.TEXT_PLAIN_CONTENT_TYPE);
			response.setHeader(SyncGlobals.DEFAULT_RECEIVE_RESPONSE_HEADER_NAME, Long.toString((new Date()).getTime()));
			response.setHeader(SyncGlobals.DEFAULT_SEND_RESPONSE_HEADER_NAME, SyncGlobals.DEFAULT_SEND_RESPONSE_HEADER_VALUE);

			bao = response.getOutputStream(); << here is the incorrect report!! (since I have that finally block)
			cos = new CountingOutputStream(bao);
			osw = new OutputStreamWriter(cos, Charset.forName(SyncGlobals.DEFAULT_CHARSET_ENCODING));

			RequestParams requestParams = new RequestParams();
			requestParams.setSyncUser(syncUser);
			manager.receive(requestParams, osw);

			cos.flush();
			bao.flush();
			response.setIntHeader("X-Content-Length", cos.getCount());
		} finally {
			if(osw != null) {
				osw.close();
			}
			if(cos != null) {
				cos.close();
			}
			if(bao != null) {
				bao.close();
			}
		}
	}
Comment 1 Diego Plentz CLA 2012-09-06 16:25:21 EDT
Reduced case:

	public void processRequest(ResponseContext responseContext) throws IOException {
		OutputStream bao = null;

		try {
			HttpServletResponse response = responseContext.getResponse();

			bao = response.getOutputStream(); <<<<
		} finally {
			if(bao != null) {
				bao.close();
			}
		}
	}
Comment 2 Stephan Herrmann CLA 2012-09-06 19:16:10 EDT
Interestingly, the very first versions of this analysis got this right (3.8M3) but since 3.8M5 we report this bogus warning.
Comment 3 Stephan Herrmann CLA 2012-09-15 18:29:47 EDT
Test & fix have been released for 4.3 M2 via commit fb2a7c4287d62c5ea38390868d83f476b5ce22e3.

The problem was similar to bug 386534 comment 5 (re out-of-order analysis of try-finally).
The trick was to detect when a FakedTrackingVariable is created in a finally block, and if so, connect it to the FlowContext of the corresponding try statement.
Now, when analysis thinks a resource variable is being re-assigned, we can see that the "previous" usage (from finally) was actually *after* the current location (within try).
Comment 4 Diego Plentz CLA 2012-09-15 18:42:01 EDT
Thanks Stephan :)
Comment 5 Srikanth Sankaran CLA 2012-09-18 01:52:53 EDT
Verified for 4.3 M2 using Build id: I20120917-0800
Comment 6 Stephan Herrmann CLA 2012-11-22 17:59:33 EST
*** Bug 376938 has been marked as a duplicate of this bug. ***
Comment 7 Stephan Herrmann CLA 2019-12-26 16:20:47 EST
*** Bug 400523 has been marked as a duplicate of this bug. ***