SmilyOrg / photofield

Experimental fast photo viewer.
MIT License
398 stars 7 forks source link

Crash when first writing configuration #97

Open Terrance opened 4 months ago

Terrance commented 4 months ago

Describe the bug

I initially started the app without a config file, then proceeded to add one whilst it was running -- looks like it detected this and reloaded, but then crashed:

collection changed, reloading
config path configuration.yaml
panic: close of closed channel
goroutine 171 [running]:
photofield/internal/fs.(*Watcher).Close(0xc000143320)
        /home/runner/work/photofield/photofield/internal/fs/watch.go:223 +0x45
main.watchConfig.func1()
        /home/runner/work/photofield/photofield/config.go:50 +0xff
main.watchConfig.func2()
        /home/runner/work/photofield/photofield/config.go:95 +0x262
created by main.watchConfig in goroutine 1
        /home/runner/work/photofield/photofield/config.go:62 +0x285

I'm running the prebuilt 0.15.0 Linux x86_64 release on Arch Linux.

Full log ``` photofield 0.15.0 config path configuration.yaml config read failed (using defaults) for configuration.yaml: open configuration.yaml: no such file or directory geo using file:data/geo/geoBoundariesCGAZ_ADM2_s5_twkb_p3.gpkg?vfs=vfs1&mode=ro unable to use exiftool, defaulting to goexif - no video metadata support (Could not create StayOpen: Failed starting exiftool in stay_open mode: exec: "exiftool": executable file not found in $PATH) cache database version 13, migrating if needed ffmpeg found at /usr/bin/ffmpeg thumbs database version 1, migrating if needed extensions .jpg, .jpeg, .png, .avif, .bmp, .pam, .ppm, .jxl, .exr, .cr2, .dng, .mp4 0 collections app running (api under /api) local http://127.0.0.1:8080 local http://[::1]:8080 ... config changed, reloading config path configuration.yaml database closing index metadata queue stopping index contents queue stopping geo using file:data/geo/geoBoundariesCGAZ_ADM2_s5_twkb_p3.gpkg?vfs=vfsc&mode=ro unable to use exiftool, defaulting to goexif - no video metadata support (Could not create StayOpen: Failed starting exiftool in stay_open mode: exec: "exiftool": executable file not found in $PATH) cache database version 13, migrating if needed ffmpeg found at /usr/bin/ffmpeg thumbs database version 1, migrating if needed extensions .jpg, .jpeg, .png, .avif, .bmp, .pam, .ppm, .jxl, .exr, .cr2, .dng, .mp4 1 collections Photos - 0 files indexed N/A ago collection changed, reloading config path configuration.yaml panic: close of closed channel goroutine 171 [running]: photofield/internal/fs.(*Watcher).Close(0xc000143320) /home/runner/work/photofield/photofield/internal/fs/watch.go:223 +0x45 main.watchConfig.func1() /home/runner/work/photofield/photofield/config.go:50 +0xff main.watchConfig.func2() /home/runner/work/photofield/photofield/config.go:95 +0x262 created by main.watchConfig in goroutine 1 /home/runner/work/photofield/photofield/config.go:62 +0x285 ```

To reproduce

What I did:

  1. Start in an empty directory
  2. Write a configuration file with an initial collection

...though trying it again now I can't reproduce.

Moving or removing the configuration file can cause a similar crash when Photofield tries to reload it:

config changed, reloading
config path configuration.yaml
config read failed (using defaults) for configuration.yaml: open configuration.yaml: no such file or directory
panic: send on closed channel

goroutine 636 [running]:
photofield/internal/fs.(*Watcher).run(0xc000129540)
        /home/runner/work/photofield/photofield/internal/fs/watch.go:211 +0x9ea
created by photofield/internal/fs.NewPathsWatcher in goroutine 630
        /home/runner/work/photofield/photofield/internal/fs/watch.go:72 +0x1df

I'm guessing there's a race condition kicking in here somewhere...

SmilyOrg commented 4 months ago

Good find! Thanks!

I was kind-of expecting there to be some dragons hidden in the autoreloading code, let's see how many...

Were you able to get it to work by restarting?

Terrance commented 4 months ago

Yes, all seems to be well again after a restart. đŸ™‚

Terrance commented 4 months ago

That said, looks like there might be similar trouble in the collection scanner:

...
index metadata   88% completed, 14996 loaded,  1897 pending, 65.52 / sec, 29s left
collection changed, reloading
config path configuration.yaml
index metadata   89% completed, 15127 loaded,  1766 pending, 64.65 / sec, 27s left
index metadata   90% completed, 15258 loaded,  1635 pending, 65.36 / sec, 25s left
index metadata   91% completed, 15403 loaded,  1490 pending, 70.91 / sec, 21s left
database closing
panic: send on closed channel
goroutine 208 [running]:
photofield/internal/image.(*Database).Write(...)
        /home/runner/work/photofield/photofield/internal/image/database.go:907
photofield/internal/image.(*Source).indexMetadata(0xc0004cc300, 0x0?)
        /home/runner/work/photofield/photofield/internal/image/indexMetadata.go:19 +0x286
created by photofield/internal/queue.(*Queue).Run in goroutine 146
        /home/runner/work/photofield/photofield/internal/queue/queue.go:42 +0x37a

This is from removing one subdirectory of a collection (with expand_subdirs: true) whilst scanning a different one.

SmilyOrg commented 4 months ago

Honestly I think what I really wanted was to just restart the process on every change... but that seems a bit of a sledgehammer. The alternative seems to be having it break in a thousand places though. đŸ¤”