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

Bug 319781

Summary:

Reference to java.util.Timer(String, boolean) not reported as error under 1.4

Product:

[Eclipse Project] PDE

Reporter:

Andrew Niefer <aniefer>

Component:

API Tools

Assignee:

Olivier Thomann <Olivier_Thomann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

major

Priority:

CC:

darin.eclipse, Michael_Rennie

Version:

3.6

Flags:

Michael_Rennie: review+
darin.eclipse: review+

Target Milestone:

3.6.1

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
Proposed fix	none
Proposed fix + regression test	none

Description Andrew Niefer

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

2010-07-13 15:28:16 EDT

Candidate for 3.6.1.
This can have serious runtime issues.

Comment 2 Olivier Thomann

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

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

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

2010-07-14 14:20:01 EDT

Darin, this is a candidate for 3.6.1?

Comment 6 Darin Wright

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

2010-07-14 14:40:17 EDT

Created attachment 174329 [details]
Proposed fix

Tests are running. So far, so good.

Comment 8 Olivier Thomann

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

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

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

2010-07-15 13:06:14 EDT

Mac OS tests also all passed

Comment 12 Olivier Thomann

2010-07-19 09:50:56 EDT

ok, thanks. I'll release shortly.

Comment 13 Darin Wright

2010-07-19 12:10:37 EDT

Looks good.

Comment 14 Olivier Thomann

2010-07-19 13:03:42 EDT

Released for 3.6.1.