feat(node): ref/unref for udp

[CosminPerRam]

I attempted on adding ref/unref for UDP.
I do not expect this to be merged (as I do not think what I've done is exactly what is needed for it), but suggestions and directions would be gladly appreciated.

Related:

• Issue regarding this: Error: Not implemented: udp.UDP.prototype.unref Error: Not implemented: udp.UDP.prototype.unref #20138
• cargo test integration::js_unit_tests::net_test yields OK.
• Didn't ran format and lint as I had problems running them (would format other files).

[CLAassistant]

All committers have signed the CLA.

[CosminPerRam]

Bump, any thoughts?

[bartlomieju]

@littledivy please review

[CosminPerRam]

Sorry for the linting issues, had some troubles running it at first, will fix asap when I'll get on my computer.

[littledivy]

Suggested change

	if (this.#listener) {
	this.#listener!.unref();
	}
	this.#listener?.unref();

[littledivy]

Instead of exposing public APIs for ref and unref in Deno.Datagram it should be implemented internally. Something like this:

Add a new private fields this.#recvPromise = null and this.#unrefed = false in UdpWrap.

In #recieve:

deno/ext/node/polyfills/internal_binding/udp_wrap.ts

Line 451 in 3b397ed

[buf, remoteAddr] = (await this.#listener!.receive(p)) as [

Change this line to store the current recieve promise in the field.

this.#recvPromise = this.#listener!.receive(p);
if (this.#unrefed) core.unrefOpPromise(this.#recvPromise);

[buf, remoteAddr] = (await this.#recvPromise) as [
// ...

// UdpWrap
ref() {
  if (this.#recvPromise) core.refOpPromise(this.#recvPromise);
  this.#unrefed = false;
}

unref() {
  if (this.#recvPromise) core.refOpPromise(this.#recvPromise);
  this.#unrefed = true;
}

[littledivy]

Please test the Node.js API instead of Deno.listenDatagram. It can be written in cli/tests/unit_node/dgram_test.ts

[CosminPerRam]

Hope I don't miss anything, but there is no cli/tests/unit_node/dgram_test.ts file, so I've created it.

udp ref and unref fails with

[dgram_test 009.80] [node/dgram] udp ref and unref => ./cli/tests/unit_node/dgram_test.ts:9:6
[dgram_test 009.80] error: Leaking async ops:
[dgram_test 009.80]   - 1 async operation to receive a datagram message via UDP was started in this test, but never completed. This is often caused by not awaiting the result of `Deno.DatagramConn#receive` call, or not breaking out of a for await loop looping over a `Deno.DatagramConn`. 

udp unref hangs indefinetly (test integration::node_unit_tests::dgram_test has been running for over 60 seconds)

[littledivy]

LGTM, thank you!

[bartlomieju]

@littledivy @CosminPerRam it appears there's still a failing test that leaks ops. Can you please take a look at it?

[CosminPerRam]

Unfortunately this beats me, @littledivy could you please take care of it (some afterwards explanations would be appreciated)?

[littledivy]

Suggested change

	if (this.#recvPromise) core.refOpPromise(this.#recvPromise);
	this.#unrefed = true;
	if (this.#recvPromise) core.unrefOpPromise(this.#recvPromise);
	this.#unrefed = true;

[littledivy]

Suggested change

	udpSocket.on("message", (buffer, _rinfo) => {
	resolve(Uint8Array.from(buffer));
	});
	let data;
	udpSocket.on("message", (buffer, _rinfo) => {
	data = Uint8Array.from(buffer);
	udpSocket.close();
	});
	udpSocket.on("close", () => {
	resolve();
	});

Socket needs to be closed.

[littledivy]

@CosminPerRam I opened #21777 with a different implementation that fixes bugs in this approach. The major one being that the promises returned by this.#listener are not ref/unrefable because they wrap the actual async op call.

[CosminPerRam]

Thanks, I will close this PR as that one superseeds this one.