Initialization handles without creating a Cursor instance

[ShahzaibIbrahim]

In win32_getHandle, a new cursor instance is created to retrieve the OS handle. With this change, we are able to safely retrieve the handle without creating a cursor instance.

How to Reproduce

To reproduce the issue, run the following snippet with resource tracking turned on:

-Dorg.eclipse.swt.graphics.Resource.reportNonDisposed=true

import org.eclipse.swt.graphics.*;
import org.eclipse.swt.widgets.*;

public class Snippet {
	public static void main(String[] args) throws InterruptedException  {
		Cursor cursor = new Cursor(Display.getDefault(), new ImageData(1, 1, 24, new PaletteData(0xFF, 0xFF00, 0xFF0000)), 0, 0);
		Cursor.win32_getHandle(cursor, 200);
		System.gc();
		Thread.sleep(1000);
	}
}

You will see the following error:

java.lang.Error: SWT Resource was not properly disposed
	at org.eclipse.swt.graphics.Resource.initNonDisposeTracking(Resource.java:191)
	at org.eclipse.swt.graphics.Resource.<init>(Resource.java:124)
	at org.eclipse.swt.graphics.Cursor.<init>(Cursor.java:282)
	at org.eclipse.swt.graphics.Cursor.win32_getHandle(Cursor.java:447)
	at org.eclipse.swt.snippets.Snippet.main(Snippet.java:9)

Expected Result

With this PR, there should be no NonDisposeResource error since we are now not creating a new Cursor instance to retrieve the handle.

Fixes: #2355

[github-actions]

Test Results

   546 files  ±0     546 suites  ±0   32m 47s ⏱️ + 3m 55s
 4 425 tests ±0   4 408 ✅ ±0   17 💤 ±0  0 ❌ ±0 
16 746 runs  ±0  16 619 ✅ ±0  127 💤 ±0  0 ❌ ±0 

Results for commit 6ce1c2d. ± Comparison against base commit df3188c.

♻️ This comment has been updated with latest results.

[HeikoKlare]

We now have three mutually exclusive fields (source, imageDataProvider and style). Only one of them is set, depending on how the cursor was initialized. And there are three setup... methods that can be executed depending on which of those three fields is set.

Can't we wrap that into something like a CursorHandleProvider interface, which has three implementations, each just accepting the necessary information (ImageData, ImageDataProvider or int/style) and then creating the handle based on that specific information. That would avoid that we always have two unused fields and that we need to switch logic depending on which of them is set.

[ShahzaibIbrahim]

Can't we wrap that into something like a CursorHandleProvider interface, which has three implementations, each just accepting the necessary information (ImageData, ImageDataProvider or int/style) and then creating the handle based on that specific information. That would avoid that we always have two unused fields and that we need to switch logic depending on which of them is set.

Good Idea, we can do it perhaps after we are done fixing this issue, in a separate PR?

[HeikoKlare]

Good Idea, we can do it perhaps after we are done fixing this issue, in a separate PR?

Yes, we can do that. But then we should probably leave out the addition of the style field in this PR, as it does not appear to be necessary for the fix of not creating unnecessary Cursor instances.