-
Notifications
You must be signed in to change notification settings - Fork 305
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
core dumped when subscrib to redis #209
Comments
Hello @CUEB-wyx, thanks for reporting this crash! Sorry I couldn't answer this earlier. I have a few questions for you before I can start investigating this issue:
If you could answer those this would be super helpful, since there are many versions of Webdis being used on various platforms so if I can reproduce the issue myself this would be a great first step before fixing it or suggesting a solution. Thanks! |
Thanks for your reply! I'm sorry I didn't describe my problem clearly. |
Thanks, this is perfect! And don't worry, there's no problem with your English :-) I'll look into this shortly. |
Ok, I was able to reproduce this easily. The work done in #199 should have made the Websocket code more reliable, but it turns out to be related to this change. This report also confirms that the feature is not quite ready yet to be enabled by default. I used Docker to run Wbedis under Ubuntu 18.04 and got the same error:
Running in GDB gave a backtrace:
@jessie-murray this seems related to your WS overhaul. I traced this to the following scenario:
It seems to me that the @jessie-murray Thoughts? Here's what I used to reproduce the issue: Dockerfile & repro stepsIn FROM ubuntu:18.04
RUN apt-get -y update && apt-get -y upgrade && apt-get -y --force-yes install git wget \
make gcc libevent-dev libmsgpack-dev python3 redis-server vim gdb valgrind
RUN mkdir /tmp/webdis
COPY . /tmp/webdis/
RUN cd /tmp/webdis && find src -name '*.d' -delete && find src -name '*.o' -delete && sed -i 's/-O3/-g -O0/g' Makefile && make -j8 clean all install
RUN echo "daemonize yes" >> /etc/redis/redis.conf && \
sed -i 's/"daemonize":.*true/"daemonize": false, "websockets": true/g' /etc/webdis.prod.json && \
sed -i 's#"logfile":.*#"logfile": "/dev/stderr"#g' /etc/webdis.prod.json && \
sed -i 's/bind 127.0.0.1 ::1/bind 127.0.0.1/g' /etc/redis/redis.conf && \
rm -f /var/log/redis/redis-server.log
CMD /usr/bin/redis-server /etc/redis/redis.conf >/dev/null 2>/dev/null && cat /etc/webdis.prod.json && /usr/local/bin/webdis /etc/webdis.prod.json
EXPOSE 7379 Built with: docker build -t webdis:ubuntu-18-04 -f Dockerfile.ubuntu . Ran with: docker run --rm -ti -p127.0.0.1:7379:7379 -v $(pwd):/tmp/webdis webdis:ubuntu-18-04 /bin/bash
service redis-server start
cd /tmp/webdis
make clean
make -j8 DEBUG=1
gdb ./webdis I opened |
… frame Connect, handshake, send FIN frame, disconnect. Webdis shouldn't crash.
@CUEB-wyx this was fixed by #210; please try again with the latest I'll release this in 0.19.0 pretty soon, I'm still experimenting with Docker manifests to see if I can bundle both an amd64 and an arm64 image under the same name, so it might be a few days until the fix is included in a release. The main challenge is finding a way to use manifests and still keep the same signatures as have been included since 0.12.0, and I think I'm close to having a final release script that uses both. |
- Performance: avoid redundant operations when building HTTP responses. - Fix HTTP parser bug on architectures that used unsigned "char" types. - Fix crash when receiving "FIN" WebSocket frame (#209).
This fixes an issue we also had @nicolasff ! We couldn't run webdis in websocket mode (with authenticated redis) - now runs smoothly. |
Thanks for your help.
When I use websocket to subscribe to redis, as long as I close the browser, the server will report an error and stop.
root@VM-0-6-ubuntu:/usr/local/webdis# ./webdis webdis.json
[err] buffer.c:565: Assertion buffer->refcnt > 0 failed in evbuffer_decref_and_unlock_
Aborted (core dumped)
The text was updated successfully, but these errors were encountered: