Random segfaults, lockups @ high CPU usage, and "read: unexpected return" messages

[taylorthurlow]

The below reproduction script triggers a series of different issues when I run it. It generates
a black image and then loops over a single remotely-hosted image asset and composites it some number of times with a small offset each time. Running it results in one of a few outcomes:

• A segfault (https://gist.github.com/taylorthurlow/e3c3e22b339920bace61dae586efc525, plus accompanying crash report https://gist.github.com/taylorthurlow/af4e8e72f82de5ba88481037e6abf211)
• Prints SEGV received in BUS handler and aborts
• The script freezes and loads up the CPU, and works until killed
• Prints read: unexpected return a few times and exits. Corresponding to the number of failed reads, some portion of the composited images will be the typical "broken jpg gray" but still composited into the final image.
• Occasionally, particularly with low numbers of input images, the image is rendered successfully.

The outcome does seem mostly random, but at times it does seem like the number of URLs in the input array might have some effect.

Using VIPS_CONCURRENCY=1 does not seem to have an effect.

To Reproduce

require "vips"
require "open-uri"

IMAGE_URLS = ["https://cdn.filestackcontent.com/bnTGtQw5ShqMPpxH2tMw"] * 10

image = Vips::Image.black(1500, 1500)

streams = []

IMAGE_URLS.each_with_index do |url, i|
  puts "loading #{url}"
  stream = URI.parse(url).open("rb")
  streams << stream
  source = Vips::SourceCustom.new
  source.on_read { |length| stream.read(length) }

  overlay = Vips::Image.new_from_source(source, "")

  image = image.composite2(overlay, :over, x: 50 * (i + 1), y: 50 * (i + 1))
end

image.write_to_file("output.jpg")

streams.each(&:close)

Expected behavior
No error is printed and the image is composited correctly each time it is called.

Desktop:

• macOS Sonoma 14.2.1, Mac Mini M3 Pro (apple silicon)
• Ruby 3.1, 3.2, and 3.3
• vips 8.15.1
• ruby-vips 2.2.0

[jcupitt]

Hi @taylorthurlow, that's a great reproducer, thank you. I'll investigate.

[jcupitt]

I made something like this that works in python (just because I was working on pyvips recently and I'm very familiar with the code):

#!/usr/bin/env python3

import sys
import gc
import requests
import pyvips

N_IMAGES = int(sys.argv[1])
URL = "https://cdn.filestackcontent.com/bnTGtQw5ShqMPpxH2tMw"
URLS = [URL] * N_IMAGES

session = requests.Session()
images = []
callbacks = []
sources = []

class StreamRead:
    def __init__(self, stream):
        self.stream = stream

    def read(self, size):
        data = self.stream.read(size)
        print(f"read: size = {size}, read = {len(data)}, stream = {self.stream}")
        return data


for i, url in enumerate(URLS):
    print(f"loading {url}")
    s = session.get(url, stream=True)
    stream = s.raw

    print(f"made stream {stream}")

    callback = StreamRead(stream)

    source = pyvips.SourceCustom()
    source.on_read(callback.read)
    sources.append(source)

    tile = pyvips.Image.new_from_source(source, "", access="sequential")
    images.append(tile) 

print(f"writing output.jpg ...")
pyvips.Image.arrayjoin(images, across=5, shim=20).write_to_file("output.jpg")
print(f"write done")

I can run this with any number of images and it seems to work fine.

It stops working, with a mixture or crashes and FFI errors, if you don't save the source in the sources list.

I think what's happening is that pyvips.Image.new_from_source is making a new VipsImage C object, and that C object is holding a ref to the VipsSourceCustom C object it needs. However, the python Image object is NOT holding a ref to the python SourceCustom object that wraps the VipsSourceCustom, so it is getting GCed. Next time you go through the python read method ... disaster!

pyvips has a system for holding and inheriting refs to wrapper objects (ruby-vips does too), it's just not being used here, and it obviously should be.

I had to make that annoying StreamRead wrapper class so that py could capture the stream object correctly, since py's closures are not very good. It's probably not necessary in ruby.

I'll try adding the extra refs and see if that fixes it.

[taylorthurlow]

Ah man I can't believe I didn't try that - it sure seemed like something was being GC'd but I just didn't think about the source. Keeping the sources from falling out of scope just by adding them to an array has completely solved the issue for me.

[jcupitt]

Great! I made a tiny PR which adds the ref, would you be able to test it?

#385

Hopefully your original reproducer will now work with that change. Hopefully.

[taylorthurlow]

Just re-ran my reproducer (25 image count if relevant) and it doesn't seem to fix it. First time got a freeze then a SEGV. Confirmed that I'm running the right code with Vips::VERSION which gives 2.2.1.

[taylorthurlow]

Just to confirm that I was running everything as expected, I hand edited your added code and confirmed it was being called, but the left side here is actively hung up and consuming high CPU.

[jcupitt]

I poked about and found a silly bug in reference handling :( I've fixed that and now your original reproducer runs well for me.

Would you have time for another quick test before I merge the PR?

[taylorthurlow]

Works great now!

[jcupitt]

Great! Now I just need to figure out why the CI is failing sigh

I think I'll just merge and hope it's a blip.

Thanks for the great reproducer. Finding things like this is very hard without a good test case, as I'm sure you know. I credited you in the changelog, let me know if that's not OK.