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

Deno 1.39.1 still getting stuck with Sharp #21686

Closed
oscarotero opened this issue Dec 22, 2023 · 10 comments · Fixed by #23497
Closed

Deno 1.39.1 still getting stuck with Sharp #21686

oscarotero opened this issue Dec 22, 2023 · 10 comments · Fixed by #23497
Assignees
Labels
bug Something isn't working correctly node native extension related to the node-api (.node) testing related to deno test and coverage

Comments

@oscarotero
Copy link
Contributor

oscarotero commented Dec 22, 2023

This issue #21601 is marked as fixed in Deno 1.39.1, but after upgrading Deno and testing again the same code:

import sharp from "npm:[email protected]";

Deno.test("testing", async () => {
  const content = Deno.readFileSync("./img.jpg");
  console.log(1);

  await sharp(content)
    .toBuffer();
  console.log(2);
  
  await sharp(content)
    .toBuffer();
  console.log(3);
});

The second promise is never resolved:

deno test -A testing.ts
running 1 test from ./testing.ts
testing ...
------- output -------
1
2

Version: Deno 1.39.1 (macOS)

@marcatatem
Copy link

marcatatem commented Dec 22, 2023

Confirming Oscar's findings again. Works until Deno 1.38.5 then gets stuck indefinitely.

@bartlomieju
Copy link
Member

Thanks, I'll look into that after holiday break.

@bartlomieju bartlomieju added bug Something isn't working correctly testing related to deno test and coverage node native extension related to the node-api (.node) labels Dec 23, 2023
@bartlomieju bartlomieju self-assigned this Dec 23, 2023
@ni-max
Copy link

ni-max commented Dec 26, 2023

I believe this is related to #21406

As I mentioned here you could try just the commit before that.

@bartlomieju bartlomieju assigned mmastrac and unassigned bartlomieju Jan 4, 2024
@oscarotero
Copy link
Contributor Author

Hi. Congrats for Deno v1.40!
Any plans to fix this issue anytime soon?
I had to pin the Deno version for Lume to 1.38.5 due this bug.

@mmastrac
Copy link
Contributor

@oscarotero This is on our radar right now. No timeline right this moment, but we hope to nail down what's going on here.

@oscarotero
Copy link
Contributor Author

Thanks @mmastrac for the quick answer.
I hope it is easy to fix.

@marcatatem
Copy link

Following up of this; interestingly, with Deno > v1.40, the second promise in @oscarotero initial test gets resolved. If you add a third await sharp(content).toBuffer(); though, the runner hangs and the promise never gets resolved.

To summarize, until v1.38.5, sharp works beautifully. After v1.38.5 and until v1.4.0, two successive calls to toBuffer() hang. After v1.4.0, two successive calls to toBuffer() work, but add a third and we're back to pre-v1.4.0 situation.

@mmastrac
Copy link
Contributor

This should be fixed when we bump deno_core. :)

mmastrac added a commit to denoland/deno_core that referenced this issue Apr 22, 2024
Ref denoland/deno#21686

If we had additional tasks generated while servicing these tasks we'd
eventually fail to wake the waker (which no longer exists in the task
spawner).
@oscarotero
Copy link
Contributor Author

@mmastrac Thanks so much!

@mmastrac
Copy link
Contributor

mmastrac commented Apr 22, 2024

Confirmed that #23497 fixes this test (and any number of subsequent calls):

import sharp from "npm:[email protected]";

Deno.test("testing", async () => {
  const content = Deno.readFileSync("./img.jpg");
  console.log(1);

  await sharp(content)
    .toBuffer();
  console.log(2);
  
  await sharp(content)
    .toBuffer();
  console.log(3);

  await sharp(content)
    .toBuffer();
  console.log(4);
});
# /tmp/deno test -A /tmp/test.ts 
Check file:///tmp/test.ts
running 1 test from ./tmp/test.ts
testing ...
------- output -------
1
2
3
4
----- output end -----
testing ... ok (27ms)

ok | 1 passed | 0 failed (37ms)

mmastrac added a commit that referenced this issue Apr 23, 2024
petamoriken pushed a commit to petamoriken/deno that referenced this issue Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node native extension related to the node-api (.node) testing related to deno test and coverage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants