Deadlock in photo download flow. about gphotos-sync

[gilesknap]

Hmmm this is an interesting one that I have not seen.

May be a little challenging for me to debug without being able to reproduce it myself, so if you can investigate that would be great.

Thanks for the report.

[peterjdolan]

FYI I've narrowed this down to how an exception in the file download flow is handled, which I think will be a terribly rare situation.

Specifically, the call to os.utime at

gphotos-sync/src/gphotos_sync/GooglePhotosDownload.py

Line 286 in 2c70af9

os.utime(

fails on my system because I'm running with a mounted SMB share, and the server doesn't allow the file attributes to be modified.

When an exception is thrown at that line, the download task times out (TimeoutException at

gphotos-sync/src/gphotos_sync/GooglePhotosDownload.py

Line 332 in 2c70af9

e = future.exception(timeout=timeout)

), at which point the database connection is already closed, and the program reaches a deadlock state and fails to exit.

I haven't been able to write a unit test to expose this because my local development environment is a bit messy, but I'll keep working on it. In the meantime, I'll just change the mounted filesystem to make it work.

[gilesknap]

Thanks @peterjdolan I figure I just need to trap that exception - can't do much but ignore it I guess.

[peterjdolan]

Agreed. I'm honestly not sure why it isn't already being caught, but it's not.

When I added a second general "except Exception" clause, it worked as expected.

I'm also not sure why the existing unit tests don't cover this. On my reading, they should.

[gilesknap]

@peterjdolan did you detect what the exception was that was thrown?
I'm only checking for PermissionError on that call.

I'm making a pr #499 that might help because it makes sure the thread is cleaned up when it throws an unexpected exception. But it would be nicer to trap your error lower down.

[gilesknap]

Please try out 3.2.5 when it is released and see if this fixes your issue.