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

Add ResourceHandler.setUseFileMapping(boolean) option #10852

Closed
cowwoc opened this issue Nov 3, 2023 · 9 comments · Fixed by #11071
Closed

Add ResourceHandler.setUseFileMapping(boolean) option #10852

cowwoc opened this issue Nov 3, 2023 · 9 comments · Fixed by #11071
Assignees

Comments

@cowwoc
Copy link
Contributor

cowwoc commented Nov 3, 2023

Jetty Version
12.0.3

Jetty Environment
Windows 10

Java Version
JDK 21

Question
In the past, one had to set useFileMappedBuffer=false to avoid holding file locks on Windows, but this configuration option seems to be gone. I am using the Handler API to serve static files. I don't use the servlet API at all.

When I'm developing JS code, I run Jetty for the backend and esbuild-watch for the frontend. Every time the file changes, esbuild-watch tries to re-build it; unfortunately it seems that Jetty is holding a lock and I'm not sure how to get around this.

@cowwoc cowwoc added the Question label Nov 3, 2023
@joakime
Copy link
Contributor

joakime commented Nov 3, 2023

@cowwoc how are you declaring your Base Resource? (code examples please)
Also, how are you accessing your static files? (hopefully via a Resource Base and not a ClassLoader).

@joakime
Copy link
Contributor

joakime commented Nov 3, 2023

On Jetty 9 / Jetty 10 / Jetty 11 the useFileMappedBuffer option disabled the ContentFactory from using java file mapped ByteBuffers.

On Jetty 12, the useFileMappedBuffer configuration still exists, but only for ee10 / ee9 / ee8 DefaultServlet (which you said you are not using).

If you used ResourceService on Jetty 9 / Jetty 10 / Jetty 11, you were responsible for passing in the useFileMappedBuffer option when you created the ResourceService.

On Jetty 12, the ResourceService has a tree of ContentFactory implementations, using the FileMappingHttpContentFactory (or not) at the top level determines the behavior.

@cowwoc
Copy link
Contributor Author

cowwoc commented Nov 3, 2023

@joakime

On Jetty 12, the ResourceService has a tree of ContentFactory implementations, using the FileMappingHttpContentFactory (or not) at the top level determines the behavior.

That makes sense, but how do I get ResourceService not to use FileMappingHttpContentFactory? I don't see any obvious way.

When reading the following code, keep in mind that the file being locked is inside the target/classes directory. Here is the relevant code snippet that creates the resource handlers:

ResourceHandler staticFiles = new ResourceHandler();
staticFiles.setDirAllowed(true);

if (scope.getRunMode() == RunMode.RELEASE)
	staticFiles.setEtags(true);

ResourceFactory resourceFactory = ResourceFactory.of(server);
List<Resource> resources = new ArrayList<>(3);

Path serverClassesPath = getPathOfDefaultPackage();
// Paths must be normalized; otherwise, Jetty will reject them for security reasons (symlinks can be
// redirected)
Path browserDirectory = serverClassesPath.resolve("../../../../client/browser/").normalize();

// Source code may be updated during development
Path srcPath = browserDirectory.resolve("src").normalize();
if (Files.exists(srcPath))
	resources.add(resourceFactory.newResource(srcPath));
Path browserClassesPath = browserDirectory.resolve("target/classes");
if (Files.exists(browserClassesPath))
	resources.add(resourceFactory.newResource(browserClassesPath));
Path browserJar = browserDirectory.resolve("target/client.browser-" + scope.getVersion() + ".jar");
if (Files.exists(browserJar))
	resources.add(resourceFactory.newJarFileResource(browserJar.toUri()));
staticFiles.setBaseResource(ResourceFactory.combine(resources));

log.debug("Loading files from: {}", staticFiles.getBaseResource());
return staticFiles;

Does anything jump out at you? I am seeing FileMappingHttpContentFactory.getMappedByteBuffer():118 getting invoked by the following stack trace:

getMappedByteBuffer:118, FileMappingHttpContentFactory$FileMappedHttpContent (org.eclipse.jetty.http.content)
getBytesOccupied:109, FileMappingHttpContentFactory$FileMappedHttpContent (org.eclipse.jetty.http.content)
getBytesOccupied:206, HttpContent$Wrapper (org.eclipse.jetty.http.content)
isCacheable:220, CachingHttpContentFactory (org.eclipse.jetty.http.content)
isCacheable:99, ValidatingCachingHttpContentFactory (org.eclipse.jetty.http.content)
getContent:242, CachingHttpContentFactory (org.eclipse.jetty.http.content)
getContent:87, ResourceService (org.eclipse.jetty.server)
handle:162, ResourceHandler (org.eclipse.jetty.server.handler)
handle:159, RewriteHandler$LastRuleHandler (org.eclipse.jetty.rewrite.handler)
handle:108, Rule$Handler (org.eclipse.jetty.rewrite.handler)
handle:89, HeaderPatternRule$1 (org.eclipse.jetty.rewrite.handler)
handle:108, Rule$Handler (org.eclipse.jetty.rewrite.handler)
handle:89, HeaderPatternRule$1 (org.eclipse.jetty.rewrite.handler)
handle:108, Rule$Handler (org.eclipse.jetty.rewrite.handler)
handle:143, RewriteHandler (org.eclipse.jetty.rewrite.handler)
handle:785, Handler$Sequence (org.eclipse.jetty.server)
handle:594, GzipHandler (org.eclipse.jetty.server.handler.gzip)
handle:809, ContextHandler (org.eclipse.jetty.server.handler)
handle:179, Server (org.eclipse.jetty.server)
run:649, HttpChannelState$HandlerInvoker (org.eclipse.jetty.server.internal)
onFillable:471, HttpConnection (org.eclipse.jetty.server.internal)
succeeded:322, AbstractConnection$ReadCallback (org.eclipse.jetty.io)
fillable:100, FillInterest (org.eclipse.jetty.io)
run:53, SelectableChannelEndPoint$1 (org.eclipse.jetty.io)
run:314, ThreadPerTaskExecutor$TaskRunner (java.util.concurrent)
[application code follows]

Thanks

@cowwoc
Copy link
Contributor Author

cowwoc commented Nov 3, 2023

I should clarify that I am using Jetty 12.0.3

@cowwoc
Copy link
Contributor Author

cowwoc commented Nov 3, 2023

Thanks to you pointing me in the right direction, I discovered the following workaround:

ResourceHandler staticFiles = new ResourceHandler()
{
	@Override
	protected HttpContent.Factory newHttpContentFactory()
	{
		HttpContent.Factory contentFactory = new ResourceHttpContentFactory(
			ResourceFactory.of(getBaseResource()), getMimeTypes());
		contentFactory = new VirtualHttpContentFactory(contentFactory, getStyleSheet(), "text/css");
		contentFactory = new PreCompressedHttpContentFactory(contentFactory, getPrecompressedFormats());
		contentFactory = new ValidatingCachingHttpContentFactory(contentFactory,
			Duration.ofSeconds(1).toMillis(), getByteBufferPool());
		return contentFactory;
	}
};

Meaning, I just took ResourceHandler.newHttpContentFactory() and removed the contentFactory = new FileMappingHttpContentFactory(contentFactory); line.

I hope that's safe... That said, does it make sense to add a configuration option to do this without having to subclassing ResourceHandler?

@joakime
Copy link
Contributor

joakime commented Nov 3, 2023

Meaning, I just took ResourceHandler.newHttpContentFactory() and removed the contentFactory = new FileMappingHttpContentFactory(contentFactory); line.

I hope that's safe... That said, does it make sense to add a configuration option to do this without having to subclassing ResourceHandler?

Yup, that's safe.
And it was what I was going to recommend as well.

@joakime
Copy link
Contributor

joakime commented Nov 3, 2023

The other option is to use the ResourceService directly (not via the ResourceHandler) so you can control the content factories yourself. (like how the DefaultServlets do)

I'm going to rename this as an issue to add useFileMapping option to the ResourceHandler as well.

@joakime joakime changed the title Files locked under Windows (what happened to useFileMappedBuffer?) ResourceHandler could use a .setUseFileMapping(boolean) option Nov 3, 2023
@joakime joakime changed the title ResourceHandler could use a .setUseFileMapping(boolean) option ResourceHandler could use a .setUseFileMapping(boolean) option Nov 3, 2023
@cowwoc
Copy link
Contributor Author

cowwoc commented Nov 3, 2023

Thank you!

@joakime
Copy link
Contributor

joakime commented Dec 15, 2023

PR #11071 merged

@joakime joakime closed this as completed Dec 15, 2023
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.5 - FROZEN Dec 15, 2023
@joakime joakime changed the title ResourceHandler could use a .setUseFileMapping(boolean) option Add ResourceHandler.setUseFileMapping(boolean) option Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants