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

"Segmentation fault" on Vercel when used inside Next.js getStaticProps (via Plaiceholder) #2655

Closed
joe-bell opened this issue Apr 8, 2021 · 7 comments
Labels

Comments

@joe-bell
Copy link

joe-bell commented Apr 8, 2021

First of all, thank you! Sharp was the fundamental step in the creation of plaiceholder, and I'm extremely grateful for the work you put into maintaining this library.

I recently stumbled across an issue using sharp on Vercel after Next.js implemented their WASM-based approach - I was no longer able to use sharp inside getStaticProps in Vercel's environment. I can build the project perfectly locally, but when deployed to Vercel, the following issue occurs.

In the example provided, sharp is used inside my library plaiceholder.

I appreciate this is both a question/issue that might be a bit out of your remit, but I would be really grateful any guidance you could offer here, as infra/deployments are a bit out of my area of expertise. Is there something I can add to sharp in plaiceholder to make it more portable? or is this something that could be resolved on the sharp side?

Related Issues


Are you using the latest version? Is the version currently in use as reported by npm ls sharp the same as the latest version as reported by npm view sharp dist-tags.latest?

Uses [email protected] (a peerDependency of plaiceholder)

What are the steps to reproduce?

Clone the minimal reproduction, and deploy to Vercel.

Alternatively, view the logs of the latest deployment.

What is the expected behaviour?

Given that sharp runs successfully as a custom pre-build step on Vercel, I would expect sharp to run successfully in getStaticProps

Are you able to provide a minimal, standalone code sample, without other dependencies, that demonstrates this problem?

Are you able to provide a sample image that helps explain the problem?

N/A

What is the output of running npx envinfo --binaries --system?

Running inside Vercel, I get the following:

09:18:42.578 | > npx envinfo --binaries --system
-- | --
09:18:43.661 | npx: installed 1 in 1.004s
09:18:44.051 | System:
09:18:44.051 | OS: Linux 4.14 Amazon Linux 2
09:18:44.051 | CPU: (4) x64 Intel(R) Xeon(R) CPU E5-2686 v4 @ 2.30GHz
09:18:44.051 | Memory: 24.10 GB / 29.94 GB
09:18:44.051 | Container: Yes
09:18:44.051 | Shell: 4.2.46 - /bin/bash
09:18:44.051 | Binaries:
09:18:44.051 | Node: 14.15.4 - /node14/bin/node
09:18:44.051 | Yarn: 1.22.10 - ~/.yarn/bin/yarn
09:18:44.051 | npm: 6.14.10 - /node14/bin/npm
@joe-bell joe-bell added the triage label Apr 8, 2021
@joe-bell
Copy link
Author

joe-bell commented Apr 8, 2021

Update: the Vercel team took a look at this and seem to believe it's something on the sharp side.

See vercel/next.js#23531 (comment)

@lovell
Copy link
Owner

lovell commented Apr 8, 2021

My best guess would be that this relates to the recent introduction of worker threads to Next.js.

The backtrace with __nptl_deallocate_tsd looks like something is attempting to dlclose a shared library whilst it is still in use.

https://sharp.pixelplumbing.com/install#worker-threads

The main thread must call require('sharp') before worker threads are created to ensure shared libraries remain loaded in memory until after all threads are complete.

There's some background to this at #2263.

@joe-bell
Copy link
Author

joe-bell commented Apr 8, 2021

Thanks for the details! I'm quite new to the concept of Node.js worker threads, so apologies if I'm missing something obvious here...

Would "calling before worker threads" be either:

  1. Something I add to getStaticProps():
    export const getStaticProps = async () => {
      require('sharp');
    
      // an external function which uses `sharp`
      const images = await getPlaceholders();
    
      return {
        props: { images },
      };
    };
  2. Something to add to my next.config.js?
  3. or something I need to ask the Next.js team to support on?

@lovell
Copy link
Owner

lovell commented Apr 8, 2021

I think this logic is already running inside a worker thread by the time getStaticProps() is called.

When Next.js removed sharp, they also removed the use of require('sharp') from the main thread, so we need to find a new "hook" that allows you to make the call to require('sharp') in the main thread.

I've been able to reproduce locally, and it looks like simply creating the following, basic next.config.js fixes it.

require('sharp');
module.exports = {};

If Next.js provides a build-time setupStaticPropsOnTheMainThread() hook, that might be a cleaner place to add this, but I'm not sure such a thing exists.

@joe-bell
Copy link
Author

joe-bell commented Apr 8, 2021

Wow you're absolutely right, that worked perfectly.

I'll pass on this message to the Next.js team and see what they'd recommend (config vs. the proposed hook).

Thanks so much for your help on this @lovell

@joe-bell joe-bell closed this as completed Apr 8, 2021
trustedtomato added a commit to trustedtomato/decharge that referenced this issue Aug 19, 2021
I've found the root of the problem:
lovell/sharp#2655
So I added import('sharp') to the main module,
now it works. Also added await worker.terminate()
instead of worker.unref() for more reliable debugging.
Plus, test.sh now uses pnpm run because segmentation
faults were occuring more reliably there, which
is again - good for debugging.
Changed Configuration - now one has to use
the provided interface, not the provided class,
because the latter didn't work due to circular dependencies.
Also updated documentation according to this change.
@devongovett
Copy link

@lovell is there a way to fix this so requiring sharp from the main thread isn't required? We're having this issue in Vercel with Parcel as well. All Parcel plugins run inside worker threads, and we don't really have a clean way to also require sharp from the main thread before our workers start within our plugin system.

@lovell
Copy link
Owner

lovell commented Sep 30, 2021

@devongovett Node.js uses reference counting internally when it calls dlopen and dlclose. It might be possible subvert that check and keep the count above zero to prevent the dlclose by calling require('sharp'), then removing its entry from require.cache, then requiring it again.

The logic in https://github.com/sindresorhus/import-fresh might help here, e.g. (untested):

require('sharp');
const sharp = require('import-fresh')('sharp');

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants