oak
oak copied to clipboard
Resource not release bug
in application.ts:
class Application->#handleRequest function:
async #handleRequest(
request: ServerRequest,
secure: boolean,
state: RequestState,
): Promise<void> {
const context = new Context(this, request, this.#getContextState(), secure);
let resolve: () => void;
const handlingPromise = new Promise<void>((res) => resolve = res);
state.handling.add(handlingPromise);
if (!state.closing && !state.closed) {
try {
await this.#getComposed()(context);
} catch (err) {
this.#handleError(context, err);
}
}
if (context.respond === false) {
context.response.destroy();
resolve!();
state.handling.delete(handlingPromise);
return;
}
let closeResources = true; //this is true
let response: Response;
try {
closeResources = false; // this is be modified to false,but no matter it enter catch or not catch,it be modified to false.
response = await context.response.toDomResponse();
} catch (err) {
this.#handleError(context, err);
response = await context.response.toDomResponse();
}
assert(response);
try {
await request.respond(response);
} catch (err) {
this.#handleError(context, err);
} finally {
context.response.destroy(closeResources);//in there , a lot of file descriptor not be closed by Deno.close(rid);
resolve!();
state.handling.delete(handlingPromise);
if (state.closing) {
state.server.close();
state.closed = true;
}
}
}
check out above comments, see if there has this problems.
I just ran into this bug today. This is not great when you try to build a reliable system just to figure out that things start to break in your production environment. A temporary workaround is to increase the nofiles ulimit. File descriptor exhaustion is a security issue. I don't think anyone would want to run into a DoS attack and a data loss.
The code that sets closeResouces to trueand then to false obviously does not make any sense. Looks like it was overlooked in the commit https://github.com/oakserver/oak/commit/5ad9c828368b1faa530ec481c7a445f5e11187cd#diff-ff7224ffde62f111cc1f7212d7e7da46d90d7258a99ca13c0db39857fbb19a03L390-L394. But one question still remains: When do you ever not close resources? Any insights on this? @kitsonk
Here are the steps to reproduce this:
1.) Create a large file of size MAXBUFFER_DEFAULT
deno eval 'Deno.writeTextFileSync("payload.txt", "A".repeat(1_048_576))'
2.) Create a issue_500.ts file and start the server deno run -A issue_500.ts
import { Application } from "https://deno.land/x/[email protected]/mod.ts";
const app = new Application();
app.use(async (context, next) => {
try {
await context.send({
root: Deno.cwd(),
index: 'payload.txt',
});
} catch {
await next();
}
});
console.log('http://127.0.0.1:8000')
await app.listen({ port: 8000 });
3.) Observe FD leak after visiting http://127.0.0.1:8000
lsof -awp $(pgrep -f issue_500.ts) | grep payload.txt
deno 323386 nekz 16r REG 8,5 1048576 18777583 /home/nekz/git/clone/oak/payload.txt
Deno version:
deno 1.38.1 (release, x86_64-unknown-linux-gnu)
v8 12.0.267.1
typescript 5.2.2