Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 319781 - Reference to java.util.Timer(String, boolean) not reported as error under 1.4
Summary: Reference to java.util.Timer(String, boolean) not reported as error under 1.4
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: API Tools (show other bugs)
Version: 3.6   Edit
Hardware: PC Linux
: P3 major (vote)
Target Milestone: 3.6.1   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-13 15:22 EDT by Andrew Niefer CLA
Modified: 2010-07-26 10:37 EDT (History)
2 users (show)

See Also:
Michael_Rennie: review+
darin.eclipse: review+


Attachments
Proposed fix (2.93 KB, patch)
2010-07-14 14:40 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix + regression test (5.91 KB, patch)
2010-07-14 15:42 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Niefer CLA 2010-07-13 15:22:50 EDT
See bug 318938, the p2 bundle org.eclipse.equinox.p2.artifact.repository contains a class org.eclipse.equinox.internal.p2.artifact.repository.MirrorSelector.MirrorInfo which contains a reference 

private static final Timer resetFailure = new Timer("Mirror failure timer", true); //$NON-NLS-1$

This constructor is @since 1.5.  The bundle is marked with 1.5, 1.4 and CDC/1.1 execution environments.  

This is not getting flagged as an api tooling error.
Comment 1 Olivier Thomann CLA 2010-07-13 15:28:16 EDT
Candidate for 3.6.1.
This can have serious runtime issues.
Comment 2 Olivier Thomann CLA 2010-07-13 15:47:03 EDT
We actually detect the wrong reference but we don't report it properly since it belongs to the static initializer method (clinit).
If the variable is not static, the problem is reported.
Comment 3 Olivier Thomann CLA 2010-07-13 15:47:41 EDT
But it is reported against the class name which is not ideal either. In this case we should have the right line to report the problem.
Comment 4 Olivier Thomann CLA 2010-07-14 14:19:36 EDT
(In reply to comment #2)
> We actually detect the wrong reference but we don't report it properly since it
> belongs to the static initializer method (clinit).
> If the variable is not static, the problem is reported.
I was wrong. We actually never collected references inside clinit methods. This is why we missed this error.
Comment 5 Olivier Thomann CLA 2010-07-14 14:20:01 EDT
Darin, this is a candidate for 3.6.1?
Comment 6 Darin Wright CLA 2010-07-14 14:31:55 EDT
(In reply to comment #5)
> Darin, this is a candidate for 3.6.1?

Guess I'd need to see a patch... to see how risky it is. I can't recall why clinit's were being ignored.
Comment 7 Olivier Thomann CLA 2010-07-14 14:40:17 EDT
Created attachment 174329 [details]
Proposed fix

Tests are running. So far, so good.
Comment 8 Olivier Thomann CLA 2010-07-14 15:42:21 EDT
Created attachment 174332 [details]
Proposed fix + regression test

Same patch with a regression test that checks references inside static initializer are scanned.
Comment 9 Olivier Thomann CLA 2010-07-15 10:57:48 EDT
Released for 3.7M1.
Will release for 3.6.1 once this is reviewed and approved.
Comment 10 Michael Rennie CLA 2010-07-15 11:05:27 EDT
+1. Patch is good and all tests pass. It still bugs me that I can't recall why we ignored clinits in the first place...
Comment 11 Michael Rennie CLA 2010-07-15 13:06:14 EDT
Mac OS tests also all passed
Comment 12 Olivier Thomann CLA 2010-07-19 09:50:56 EDT
ok, thanks. I'll release shortly.
Comment 13 Darin Wright CLA 2010-07-19 12:10:37 EDT
Looks good.
Comment 14 Olivier Thomann CLA 2010-07-19 13:03:42 EDT
Released for 3.6.1.