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

Crash on notification settings #6

Closed
jfjallid opened this issue Oct 28, 2016 · 25 comments · Fixed by #8
Closed

Crash on notification settings #6

jfjallid opened this issue Oct 28, 2016 · 25 comments · Fixed by #8
Assignees

Comments

@jfjallid
Copy link

Whenever I try to access the notification settings the docker container crashes without any error messages. I'm running the container with your example command but with modified values for paths, uid and gid

@ressu
Copy link
Contributor

ressu commented Oct 30, 2016

Which platform are you running the image on? Crashing without any messages makes me think of a recent Docker bug that could be causing the crash.

@ressu ressu self-assigned this Oct 30, 2016
@jfjallid
Copy link
Author

I'm running it on CentOS x64.
Docker version 1.12.1, build 23cf638.

@jfjallid
Copy link
Author

I've tried running it on Docker version 1.12.3, build 6b644ec and the problem remains.
When I run the developer image instead, the webserver crashes but the docker container keeps running. The webserver crashes with the message "The connection was reset" and no further output in the container log. I'm running the container interactively and after the webserver crashes it just hangs silently without writing anything else to stdout or stderr.

@ressu
Copy link
Contributor

ressu commented Oct 31, 2016

OK, I'll try and reproduce tonight.

@ressu
Copy link
Contributor

ressu commented Oct 31, 2016

I'm able to reproduce the issue. There doesn't appear to be any messages in the log or console. I'll try and dig deeper and try and find out what is causing the exit.

@jfjallid
Copy link
Author

@ressu Any progress on this?

@ressu
Copy link
Contributor

ressu commented Dec 27, 2016

I'm having a hard time getting this resolved, it feels a lot like an musl compatibility issue. I'm currently trying to narrow down what is actually causing the crash and working backwards from there

ressu pushed a commit to ressu/SickGear-docker that referenced this issue Jan 30, 2017
@ressu
Copy link
Contributor

ressu commented Jan 30, 2017

The fixed builds should now be up. Took me a bit longer to get around to fixing this than I expected.. The only solution I could find was to switch away from the Alpine based image to Debian based image.

JackDandy pushed a commit that referenced this issue Feb 1, 2017
@jfjallid
Copy link
Author

jfjallid commented Feb 5, 2017

Yes, this solved the problem nicely. Thanks!

@deed02392
Copy link
Contributor

deed02392 commented Jul 22, 2017

@ressu this can be fixed by setting the threading stack size in SickGear. Thus we can continue to use Alpine as the base.

These are the refs to the root cause of the issue:

python-pillow/Pillow#1935
python-pillow/Pillow#1935 (comment)
docker-library/python#211 (comment)
https://github.com/voidlinux/void-packages/issues/4147
https://github.com/voidlinux/void-packages/pull/6889/files

@ressu
Copy link
Contributor

ressu commented Jul 26, 2017

Cool, I personally prefer Alpine over Debian for the Docker image. Pull requests are welcome, I'm not sure when I'm able to revert PR #8 and test with the new threading stack sizes.

@deed02392
Copy link
Contributor

So I've spoken with @JackDandy and we're working on testing a config directive for specifying the default thread stack size. That way we can revert to an Alpine based docker image in master, and just update the config template to specify the stack size to something large enough to keep it working. I haven't yet managed to test the PR over on kulta regarding this new config setting though.

@timm088
Copy link

timm088 commented Aug 10, 2017

Thanks for looking into the fix, however, shouldn't this one still be open ? Took me a little to dig it up is all

@deed02392
Copy link
Contributor

@timm088 I basically investigated and resolved the issue entirely independently before realising it was fixed on master, because there wasn't an open issue :(
@ressu I do think you should make it clearer to people that this is a problem on dev with a change to the dev branches README. And look at my PR/harass @JackDandy for input :-)

@timm088
Copy link

timm088 commented Aug 12, 2017

Ok thanks. Would be great if that pr could be linked here, so I can just pulled it myself?

@deed02392
Copy link
Contributor

deed02392 commented Aug 13, 2017 via email

@timm088
Copy link

timm088 commented Aug 14, 2017

Last commit on master was

9c3a158a5df3aea9f438e3038074282e2fbe5f44
Confirmed my alpine docker container has this same commit in Sickgear / Settings / About

Problem still exists for Alpine docker images on current master branch

No problems if it isn't merged yet, I manually configured notifications via config.ini (which for me is mounted into the container each time)

@deed02392
Copy link
Contributor

I can't find the commit you're referring to... This is the latest commit on the master branch:

16a847c

@timm088
Copy link

timm088 commented Aug 14, 2017

Sorry, i'm referring to the master branch of Sickgear itself, not the dockerfile

i.e., you said its fixed on master? You mean changed to debian, not actually fixed (i.e, threading stack size fixed)

@deed02392
Copy link
Contributor

If you just want to change the SickGear version to fix this issue, you can use this commit from the current hotfix branch: 70edd5462fca9ebdfc4ffa6caef7708219e79704

Then set the stack_size = 327680 under General in config.ini

@timm088
Copy link

timm088 commented Aug 15, 2017

Hotfix branch?
There are only two visible branches...

@deed02392
Copy link
Contributor

Did you actually find the commit I referred to?

@timm088
Copy link

timm088 commented Aug 15, 2017

You do realise there is no hotfix branch right?
https://github.com/SickGear/SickGear/branches/all

Guessing hotfix branch is private?

You mean this commit? SickGear/SickGear@70edd54

@deed02392
Copy link
Contributor

https://git.kulta.us/SickGear/SickGear/src/hotfix/0.12.26

That's the right commit yes.

@timm088
Copy link

timm088 commented Aug 27, 2017

Merged into master on github now for anyone else that might come across this.
SickGear/SickGear@2c2a5b1

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

Successfully merging a pull request may close this issue.

5 participants