Skip to content

Conversation

@vi
Copy link
Member

@vi vi commented Nov 24, 2016

Hacks:

  • Dummy SslContext
  • Trickery in secure server doctest
  • Sha1 used from other crate instead of openssl (even openssl enabled)

vi added 2 commits November 24, 2016 02:32
Hacks:

* Dummy SslContext
* Trickery in secure server doctest
* Sha1 used from other crate instead of openssl (even openssl enabled)
@bgourlie
Copy link

bgourlie commented Dec 1, 2016

Maintainer seems to be MIA. Maybe time to fork?

@vi
Copy link
Member Author

vi commented Dec 2, 2016

@bgourlie, Are you interested in this and/or #91 being on crates.io sooner?

My next ideas for rust-websocket:

  • Custom transport to make possible for users to implement websocket atop of UNIX sockets (to serve as Nginx backend more nicely)
  • Building atop of alternative SSL with rustls and hyper-rustls, so easier cross-compilation.

I currently have a reminder for thinking about forking rust-websocket if no maintainer replies set at 2017-02-01.

P.S.: Implemented by app "websocat" using this library. Pre-built versions are with the pull requests included.

@bgourlie
Copy link

bgourlie commented Dec 2, 2016

I don't have an urgent need for this to be on crates.io. I'm currently using this crate as a stop-gap measure in a personal project, and not being able to compile on Windows due to the open-ssl dependency is a minor pain.

Long-term, I'd like something built on top of tokio. I've been been waiting for the tokio ecosystem to stabilize before attempting to build a websocket library on top of it, but it sounds like that time might be near.

I think that forking this library for quality-of-life improvements is still worth it (assuming the original maintainer doesn't suddenly appear), but I really think that the rust ecosystem needs a websocket library built on top of futures/tokio.

@bgourlie
Copy link

So I attempted to use this branch, turning off the default ssl dependency, and I get a stackoverflow when running my app. Any idea what could cause that?

@vi
Copy link
Member Author

vi commented Jan 29, 2017

Note: my fork of websocket is called websocket-vi.

Do you have a backtrace? I don't imagine how usage nossl mode can produce stack overflow.

Do you have minimal example of reproducing the problem?

@bgourlie
Copy link

So this is how I'm currently configuring the websocket dependency:

websocket = { git = "https://github.com/vi/rust-websocket.git", branch = "optional_openssl", default-features = false }

Unfortunately, there is no backtrace. It's a hard crash on Windows, hangs indefinitely on Linux:

capture

Same results using the vi branch (after renaming all references to websocket to websocket_vi.

I can try to provide a minimal reproduction, but it will take me a while. In the meantime you can see how I'm using it in practice here.

I looked over the changes made in your optional_ssl branch and I agree that there is nothing obvious that could be causing this but alas, everything works with the websocket package from crates.io, except that it doesn't compile on Windows thanks to the openssl dependency.

@bgourlie
Copy link

Before you put any effort into looking into this, let me do a few more things for due diligence.

@bgourlie
Copy link

So, after a little more research:

  • It's only affecting windows
  • I checked out your vi branch and ran the client/server examples on Windows and they work fine.

So, I'm doing something funky here that is triggering this issue, and it only affects Windows. I'll try to find a minimal repro.

@vi
Copy link
Member Author

vi commented Jan 29, 2017

You can now use websocket-vi from crates.io with default-featues off to get nossl.

@bgourlie
Copy link

bgourlie commented Jan 29, 2017

It will be nice to have an actively maintained version of this library. Are you able to have your own issue tracker on a cloned repository? That way we could move the discussion away from this repo.

@vi
Copy link
Member Author

vi commented Jan 29, 2017

Oh, forgot that issues are disabled by default on forks...

Done

@vi
Copy link
Member Author

vi commented Jan 29, 2017

@bgourlie Are you explicitly interested in #80 or #93 ?

@bgourlie
Copy link

Neither of those are really needed for my particular use-case. My ideal websocket library would be built on top of tokio, mostly because have a single thread managing all websocket events in conjunction with tokio's channel implementation would greatly simplify my code. It's something I'd like to eventually start implementing, but I'm waiting for tokio, et-al to stabilize a bit.

@vi
Copy link
Member Author

vi commented Jan 29, 2017

I never tried Tokio myself yet.

Basically I dealed with rust-websocket only for websocat; and ws was not suitable because of lack of backpressure. Uness it is trivial-ish, by the time I get round to actually implement rust-websocket for tokio somebody else may already do it.

@bgourlie
Copy link

Tokio is working on a backpressure strategy for their next release. If you haven't already, I'd suggest reading the original tokio announcement.

@illegalprime
Copy link
Collaborator

@vi thanks for the work, I'll look over these soon

@vi
Copy link
Member Author

vi commented Mar 27, 2017

Note: I've already forked and published it under the name websocket-vi

If necessary changes (including some analogue of vi#2) get integrated here then the fork may be discontinued.

@illegalprime
Copy link
Collaborator

@vi yeah I saw, would you be open to merging the two projects?

@vi
Copy link
Member Author

vi commented Mar 27, 2017

@illegalprime , Yes. All advancements should already be in pull requests (except of the link in previous comment).

@illegalprime
Copy link
Collaborator

@vi love to talk about ideas for the future of this crate since you mentioned you had some

@illegalprime
Copy link
Collaborator

@vi @bgourlie all these features are in master now, closing this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants