Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No more handle exceptions with the Edge browser #1013

Open
vogella opened this issue Jan 29, 2024 · 17 comments · May be fixed by #1548
Open

No more handle exceptions with the Edge browser #1013

vogella opened this issue Jan 29, 2024 · 17 comments · May be fixed by #1548
Labels
edge Edge Browser Windows Happens on Windows OS

Comments

@vogella
Copy link
Contributor

vogella commented Jan 29, 2024

Using the Edge browser on Windows still results in a the no more handlers exception even after the commit ed8f821 from @HeikoKlare

Before the commit, the error was in Edge.java:343, now it occurs in Edge.java:362.

https://github.com/eclipse-platform/eclipse.platform.swt/blob/master/bundles/org.eclipse.swt/Eclipse%20SWT%20Browser/win32/org/eclipse/swt/browser/Edge.java#L362C1-L363C1

It seems to happen if the user starts multiple time the same application in different workspaces. I will try to provide a test case to reproduce.

@vogella vogella changed the title No more handle exceptions No more handle exceptions with the Edge browser Jan 29, 2024
@vogella
Copy link
Contributor Author

vogella commented Jan 29, 2024

See #339

@HeikoKlare
Copy link
Contributor

I am sorry that Edge instantiation does still not work properly. We did not have issues in productive usage recently, but we do not use it intensively either (usually only one instance). Some remarks on this:

I could image that the problem you describe, @vogella, is related to the one that can be seen in the tests.
What I tried to do in my previous fix attempt was to ensure that asynchronous OS events for browser instantiation are processed properly. Taking a fresh look at the code, I would first have a look at the involved Edge#callAndWait() implementations again:

static int callAndWait(long[] ppv, ToIntFunction<IUnknown> callable) {
int[] phr = {COM.S_OK};
IUnknown completion = newCallback((result, pv) -> {
phr[0] = (int)result;
if ((int)result == COM.S_OK) {
ppv[0] = pv;
new IUnknown(pv).AddRef();
}
return COM.S_OK;
});
ppv[0] = 0;
phr[0] = callable.applyAsInt(completion);
completion.Release();
while (phr[0] == COM.S_OK && ppv[0] == 0) {
processNextOSMessage();
}
return phr[0];
}
static int callAndWait(String[] pstr, ToIntFunction<IUnknown> callable) {
int[] phr = new int[1];
IUnknown completion = newCallback((result, pszJson) -> {
phr[0] = (int)result;
if ((int)result == COM.S_OK) {
pstr[0] = wstrToString(pszJson, false);
}
return COM.S_OK;
});
pstr[0] = null;
phr[0] = callable.applyAsInt(completion);
completion.Release();
while (phr[0] == COM.S_OK && pstr[0] == null) {
processNextOSMessage();
}
return phr[0];
}

From my understanding, the OS event processing is supposed to trigger the callback setting the results (ppv/phr). However, the callAndWait performs a release on the callback even before doing the event processing, so maybe the problem is something pretty simple like an interface pointer being released too early.
completion.Release();
while (phr[0] == COM.S_OK && ppv[0] == 0) {
processNextOSMessage();
}

HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 30, 2024
…ipse-platform#1013

Instead of always showing a "no more handles" error, with this change an
error due to invalid thread access is explicitly logged as such.

Contributes to
eclipse-platform#1013
@HeikoKlare
Copy link
Contributor

@vogella can you share which kind of error code is printed? I.e., there should be a message like
org.eclipse.swt.SWTError: No more handles [0x802a000c]
and the error code at the end of the message would be of particular interest. For error code 0x802a000c (wrong thred access), I have provided #1015 to better distinguish that kind of error.

HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 30, 2024
…ipse-platform#1013

The handle to a newly instantiated Edge browser is provided via a
callback. This callback may be either be invoked synchronously when
creating a WebView2 controller (happens on first creation of an Edge
browser instance in an application) or asynchronously via a processed OS
event (from second creation of an Edge browser instance onwards). The
callback is currently released before the OS events are processed, which
may cause issues when instantiating more than one Edge browser in a
single application.

With this change, the callback is only released when it is not required
and may not be called anymore. A test instantiating multiple (Edge)
browsers in a single application is added to ensure that the
asynchronous callback invocation is executed in a test.

Contributes to
eclipse-platform#1013
HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 30, 2024
…ipse-platform#1013

Instead of always showing a "no more handles" error, with this change an
error due to invalid thread access is explicitly logged as such.

Contributes to
eclipse-platform#1013
HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 30, 2024
…ipse-platform#1013

The handle to a newly instantiated Edge browser is provided via a
callback. This callback may be either be invoked synchronously when
creating a WebView2 controller (happens on first creation of an Edge
browser instance in an application) or asynchronously via a processed OS
event (from second creation of an Edge browser instance onwards). The
callback is currently released before the OS events are processed, which
may cause issues when instantiating more than one Edge browser in a
single application.

With this change, the callback is only released when it is not required
and may not be called anymore. A test instantiating multiple (Edge)
browsers in a single application is added to ensure that the
asynchronous callback invocation is executed in a test.

Contributes to
eclipse-platform#1013
@vogella
Copy link
Contributor Author

vogella commented Jan 31, 2024

We see two errors:
No more handles [0x8007139f]
No more handles [0x800700aa]

[0x8007139f] happens more frequently.

vogella pushed a commit that referenced this issue Jan 31, 2024
Instead of always showing a "no more handles" error, with this change an
error due to invalid thread access is explicitly logged as such.

Contributes to
#1013
vogella pushed a commit that referenced this issue Jan 31, 2024
The handle to a newly instantiated Edge browser is provided via a
callback. This callback may be either be invoked synchronously when
creating a WebView2 controller (happens on first creation of an Edge
browser instance in an application) or asynchronously via a processed OS
event (from second creation of an Edge browser instance onwards). The
callback is currently released before the OS events are processed, which
may cause issues when instantiating more than one Edge browser in a
single application.

With this change, the callback is only released when it is not required
and may not be called anymore. A test instantiating multiple (Edge)
browsers in a single application is added to ensure that the
asynchronous callback invocation is executed in a test.

Contributes to
#1013
@HeikoKlare
Copy link
Contributor

Thanks for sharing!

Some documentation I found (including the Microsoft documentation) states the following:

  • 0x800700AA (ERROR_BUSY): May be used to indicate that the device is busy processing another operation. Applications should wait for that operation to complete before retrying.
  • 0x8007139F: The group or resource is not in the correct state to perform the requested operation (also documented in a WebView2 issue)

The question is what kind of concurrent operation or incorrect state that is. One thing I could imagine is the access to a shared data directory: all Edge instances within (Eclipse) applications that have the same application name share the same data directory with the following path:

appLocalDir = System.getenv("LOCALAPPDATA") + "\\" + Display.APP_NAME.replaceAll("[\\\\/:*?\"<>|]", "_");

This will be something like C:\Users\...\AppData\Local\Eclipse.

@vogella is it possible for you to set the system property org.eclipse.swt.browser.EdgeDataDir to something different for each application and test if the issue still occurs? This will result in a different data folder for each application and would allow to verify whether the shared data directory is related to the issue.

@vogella
Copy link
Contributor Author

vogella commented Jan 31, 2024

@HeikoKlare thanks for the information, we will try the org.eclipse.swt.browser.EdgeDataDir setting and I will report back if that solves the issue.

HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 31, 2024
The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
eclipse-platform#1013
HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 31, 2024
The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
eclipse-platform#1013
@vogella
Copy link
Contributor Author

vogella commented Jan 31, 2024

JVM shutdown hook, via

Runtime.getRuntime().addShutdownHook(new Thread(() -> deleteDirectoryRecursively(edgeDataDir)));

private static void deleteDirectoryRecursively(Path path) {

		try {
			Files.walk(path).sorted((a, b) -> b.compareTo(a)) // Sort in reverse order to delete files before
																// directories
					.forEach(p -> {
						try {
							Files.delete(p);
						} catch (IOException e) {
							e.printStackTrace();
						}
					});
		} catch (IOException e) {
			e.printStackTrace();
		}
	}

works fine now (not sure what was wrong before).

And we currently do not see the no more handlers exception. Thanks @HeikoKlare

@HeikoKlare
Copy link
Contributor

Good to hear that setting the data directory may solve/circumenvent the problem. Still this is quite interesting as meanwhile I took a look into Microsoft's documentation which states that in case you configure multiple WebView2Environments equally, they should be able to share a data directory: https://learn.microsoft.com/en-us/microsoft-edge/webview2/concepts/process-model?tabs=csharp#multiple-environment-objects
So maybe there is some further difference in WebView2Options used by different SWT Edge instances (even though it looks as if their default values should be equal).

Anyway, I would expect that different RCP applications do not share browser data by default. I would rather expect an RCP application to place the browser data directory within its workspace folder. So maybe in case the user does not set a custom data directory, an RCP application should explicitly do so?

a dispose listener on Display cannot delete as the files are still locked

Might be that this is a race condition, since the WebView2Environment handle is also released via a display dispose listener:

display.disposeExec(() -> {
Environment.Release();
Environment = null;
});

HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 31, 2024
The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
eclipse-platform#1013
HeikoKlare added a commit to HeikoKlare/eclipse.platform.swt that referenced this issue Jan 31, 2024
The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
eclipse-platform#1013
HeikoKlare added a commit that referenced this issue Jan 31, 2024
The current implementation of the Edge browser creates a
WebView2Environment on first creation of an WebView2 / Edge browser
instance. On every further creation of a WebView2 instance, this
environment is used. In case the WebView2 instance is created for a
different display, i.e., within a different thread, the instantiation
fails as the WebView2Environment has been created in a different thread.

To support the creation of WebView2 instances in different displays,
this change replaces the static WebView2Environment with one environment
for every display. The WebView2 instances are created for the
environment belonging to the display for which the current instantiation
is requested. An according regression test is added.

Contributes to
#1013
@vogella
Copy link
Contributor Author

vogella commented Feb 1, 2024

Thanks @HeikoKlare for your work.

We tested your changes via the i-build from yesterday and it seems to solve our errors event without the edge data setting in most cases.

We tested: Starting multiple production RCP applications in parallel with and without the edge data directory set.

It still gives the error: No more handles [0x8007139f] if we do NOT set the edge data directory and start the application via the IDE and have another production version already running (which we build via Tycho). So a runtime version seems still to collide with a build version.

@HeikoKlare
Copy link
Contributor

Thanks for the follow-up information!
It is interesting that multiple production applications seem to work fine while a production version infers with a version started from within Eclipse. And I have to admit that I have no clue why that happens. Maybe it is related to the used application names, as they are used to determine the default data directory.
Anyway, there is pretty much to consider with respect to (sharing) user data directories: https://learn.microsoft.com/en-us/microsoft-edge/webview2/concepts/user-data-folder?tabs=win32

To me, it still seems to make sense to have the data folder either placed in the installation folder or even within the workspace (metadata) folder:

Anyway, I would expect that different RCP applications do not share browser data by default. I would rather expect an RCP application to place the browser data directory within its workspace folder. So maybe in case the user does not set a custom data directory, an RCP application should explicitly do so?

Then applications / workspaces are isolated properly. But it also feels a little strange to integrate such specific behavior into the general framework and maybe that should be something each application decides on its own. Then we could only think about adding it to the IDEApplication.

@laeubi
Copy link
Contributor

laeubi commented Feb 1, 2024

@HeikoKlare using the installfolder is maybe not a good idea as one can still start multiple eclipse from the same install but different workspaces. Also the install location might be read-only.

@HeikoKlare
Copy link
Contributor

using the installfolder is maybe not a good idea as one can still start multiple eclipse from the same install but different workspaces. Also the install location might be read-only.

That's true. I also think that the workspace directory makes much more sense (semantically). Technically, it should be possible that multiple Eclipse instances started from the same installation use the same, shared data folder (according to the WebView2 documentation). But even if that worked properly, you are right that the install folder might be read-only and then you need some fallback anyway.

@jcompagner
Copy link

just to share ours stuff that we get when enabling the Edge browser:

org.eclipse.swt.SWTError: No more handles [0x80070578]
	at org.eclipse.swt.SWT.error(SWT.java:4944)
	at org.eclipse.swt.browser.Edge.error(Edge.java:190)
	at org.eclipse.swt.browser.Edge.create(Edge.java:362)
	at org.eclipse.swt.browser.Browser.<init>(Browser.java:99)
	at com.servoy.eclipse.ui.views.solutionexplorer.HTMLToolTipSupport.createBrowserTooltipContentArea(HTMLToolTipSupport.java:80)
	at com.servoy.eclipse.ui.views.solutionexplorer.HTMLToolTipSupport.createViewerToolTipContentArea(HTMLToolTipSupport.java:65)
	at org.eclipse.jface.viewers.ColumnViewerToolTipSupport.createToolTipContentArea(ColumnViewerToolTipSupport.java:109)
	at org.eclipse.jface.window.ToolTip.toolTipShow(ToolTip.java:345)
	at org.eclipse.jface.window.ToolTip.toolTipOpen(ToolTip.java:469)
	at org.eclipse.jface.window.ToolTip.toolTipCreate(ToolTip.java:334)
	at org.eclipse.jface.window.ToolTip$ToolTipOwnerControlListener.handleEvent(ToolTip.java:609)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89)
	at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4273)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1066)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4071)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3659)

that is what we get all the time when we use a tooltip that uses the browser
i first thought, maybe i forget to dispose something? But then i noticed that it is already in the first call to that method that goes wrong.

Then i tried to debug it a bit, and that got me to a very surprising behavior

image

that is the Edge class if i place a break point at 361, when it hits an i press resume..
bang "no more handles"
if i step over (or step in and step out) so i really try to go to line 362..
it works... 100% of the time..

jcompagner added a commit to Servoy/servoy-eclipse that referenced this issue Feb 8, 2024
@HeikoKlare
Copy link
Contributor

Error code 0x80070578 means: "Invalid window handle"

Can you please post the environment (OS version, WebView2 version) on which the problem occurs? Doing a web search for that error code in combination with WebView2 gives several results over the last years, e.g., MicrosoftEdge/WebView2Feedback#3097

@jcompagner
Copy link

this is on all our windows environments (Windows 11). on our project build on Eclipse 2023.12 (4.30)

WebView2 version? i have no idea. that is default i think installed in windows 11, how can i quickly test that?

@HeikoKlare
Copy link
Contributor

The WebView2 runtime is installed as an ordinary Windows applications, so within the programs overview, you should find the runtime:
image

I had a quick look into the class instantiating the browser, but unfortunately the posted strack trace does seem to fit with the source code.

@jcompagner
Copy link

i have the same version

it is this line:

https://github.com/Servoy/servoy-eclipse/blob/master/com.servoy.eclipse.ui/src/com/servoy/eclipse/ui/views/solutionexplorer/HTMLToolTipSupport.java#L76

in that class so just new Browser()

my code was a bit different because of some debug stuff and dispose try outs.

@HeikoKlare HeikoKlare added Windows Happens on Windows OS edge Edge Browser labels Jul 26, 2024
amartya4256 added a commit to amartya4256/eclipse.platform.ui that referenced this issue Oct 21, 2024
This contributes to adding the workspace address to the display data on
the initilization of the workbench. This contribution allows further SWT
components to utilize this information.

Contributes to eclipse-platform/eclipse.platform.swt#1013
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Oct 21, 2024
With this contribution, Edge browser uses the workspace directory for
the userdata directory for the separation of the usage of userdata
directory per workspace by the webview2Environment.

contributes to eclipse-platform#1013
@fedejeanne fedejeanne linked a pull request Oct 25, 2024 that will close this issue
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Oct 25, 2024
With this contribution, Edge browser uses the workspace directory for
the userdata directory for the separation of the usage of userdata
directory per workspace by the webview2Environment.

contributes to eclipse-platform#1013
amartya4256 added a commit to amartya4256/eclipse.platform.ui that referenced this issue Oct 25, 2024
This contributes to adding the workspace address to the display data on
the initilization of the workbench. This contribution allows further SWT
components to utilize this information.

Contributes to eclipse-platform/eclipse.platform.swt#1013
amartya4256 added a commit to amartya4256/eclipse.platform.swt that referenced this issue Oct 25, 2024
With this contribution, Edge browser uses the workspace directory for
the userdata directory for the separation of the usage of userdata
directory per workspace by the webview2Environment.

contributes to eclipse-platform#1013
amartya4256 added a commit to amartya4256/eclipse.platform.ui that referenced this issue Oct 25, 2024
This contributes to adding the workspace address to the display data on
the initilization of the workbench. This contribution allows further SWT
components to utilize this information.

Contributes to eclipse-platform/eclipse.platform.swt#1013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
edge Edge Browser Windows Happens on Windows OS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants