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.

@kitsonk

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 5ad9c82#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/oak@v12.6.1/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

Attempt to fix DoS issue oakserver/oak#500

Fix redirect regression

Navigation Menu

Repository navigation

Resource not release bug #500

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Resource not release bug #500

Description

Activity

NeKzor commented on Nov 14, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions