darrenjs / wampcc

WAMP C++ library
MIT License
73 stars 25 forks source link

wampcc only runs against the version of libuv that it was built against #51

Closed samangh closed 5 years ago

samangh commented 5 years ago

Hi Darren,

Thanks for your work onwampcc. I'm currently planning to use it as part of a project, so hopefully will get the chance to send some pull requests :-).

It seems that wampcc will only run if linked against the specific version of libuv that it was compiled against. Whilst this works fine when running on the machine that its compiled on, this becomes an issued when deploying wampcc on machines with different version of libuv.

https://github.com/darrenjs/wampcc/blob/247d7b11509423f001206a45c34510da7ceff135/libs/wampcc/io_loop.cc#L58

May I ask why you have this check in there?

Thanks, Saman

darrenjs commented 5 years ago

Hi Saman,

Thanks for interest in wampcc.

Well, I think that check has been added to test that the version of libuv found at runtime is the same version that was used at compile time. Generally I believe this important to do, because mixing different versions at compile-time and run-time will lead to undefined behaviour, i.e, polite way of saying a core-dump. In the simplest case, the size of variables of libuv types in wampcc code that get compiled-in reflect the headers found at compile time; these variables may then be passed into libuv functions at runtime, and the runtime code might expect different sizes and members. So it is critical for there to be compatibility between compile-time libuv headers and runtime libuv.so

What I would suggest is that if you build an application that uses wampcc, and you wish to distribute to run on other machines, then either perform static linking (i..e integrate libuv into your app), or, ship the libuv libraries with your application. And the same goes for the Json library, jansson. E.g., you could like dynamically to wampcc, and then ship wampcc.so, jansson.so and libuv.so all in the same package.

Hope that helps,

Darren

samangh commented 5 years ago

Dear Darren,

Apologies for my delayed reply.

I do understand your point with regards to unexpected changes in the ABI if the version of libuv that wampcc is compiled against is different to the version at runtime. I think, however, that in the Linux world this problem is generally solved because the linker always checks that SOVERSION of the library that was used at compile time matches that of the runtime library.

For example, if versions 1.0 to 1.5 of a shared library libfoo provide identical interfaces, then the libraries have the same name (libfoo.so.1). If on the other hand version 2 of libfoo is no longer backwards compatible, then the soname would be changed to libfoo.so.2 and any binary that was compiled against the incompatible version 1 of that library would fail to load. This scheme can carry to minor and patch numbers as well (e.g. libfoo.so.1.2.3). Please see here, here and here.

This generally works well as long as libraries change their SOVERSION when they make breaking changes. I assume that libuv is following this convention. Otherwise one could take this to the extreme and include all the libraries that a program uses all the way down to libc.so.

For example, the wampcc compiled on my system is linked against libuv.so.1 and would fail to load if an incompatible version of the library (for example libuv.so.2) was used instead.

$ ldd /usr/lib/libwampcc.so
        linux-vdso.so.1 (0x00007ffc84597000)
        libuv.so.1 => /lib/x86_64-linux-gnu/libuv.so.1 (0x00007f8976f6f000)
        libssl.so.1.1 => /lib/x86_64-linux-gnu/libssl.so.1.1 (0x00007f8976ede000)
        libwampcc_json.so.6 => /lib/libwampcc_json.so.6 (0x00007f8976ec1000)
        ...

In Debian at the moment all the following programs are using libuv1 and depend on the SOVERSION to ensure compatibility:

$ apt-cache rdepends libuv1

libuv1
Reverse Depends:
  nodejs
  python3-uvloop-dbg
  python3-uvloop
  siridb-server
  rakudo
  libradare2-3.2
  r-cran-httpuv
  r-cran-fs
  passenger
  libuv1-dev
  libnode64
  neovim
  moarvm
  lua-nvim
  lua-luv
  libwebsockets8
  libwebsockets-test-server
  cmake
  storj
  libstorj0
  knot-resolver
  hddemux
  libh2o0.13
  libgetdns10
  cmake-qt-gui
  cmake-curses-gui

So I believe that for Linux releases at least, it makes sense to rely on the SOVERION to take care of library compatibility. I'm not sure about Windows however, so how do you feel about putting this check in-between a #if defined(_WIN32) ... #endif check?

In any case, thank you again for placing wampcc on Github.

Best, Saman

darrenjs commented 5 years ago

Hi ... thanks for your explanation. I think want you say makes a lot of sense, so I'll review the code to see if it can be dropped.

darrenjs commented 5 years ago

Hi ... I agree with you, it is a bit restrictive to expect the run-time and compile-time versions to be exactly the same. I am going to change this. The background for this feature is when libraries like wampcc are used by developers who are not very good at dependency management (e.g. most often I see developers not really understanding dependency management, and they tend to ship a binary with all of its libraries), this check acted as a safety precaution for them.

So my plan is to move the check. It will no-longer get called automatically, and won't throw exception. Instead I'll change it to just return compile-time and run-time versions, and it will be up to the user to invoke it and act on the values returned (e.g. just log, or error-out). I.e. its demoted to an optional reflection method.

samangh commented 5 years ago

Thanks for looking into this. I do understand the original thinking, and I think your solution of returning the compile and run-time versions is reasonable.

Changes in the libuv ABI has been tracked here: https://abi-laboratory.pro/?view=timeline&l=libuv. Looks like it has been pretty stable since 1.0.0 :-).

darrenjs commented 5 years ago

Addressed this in commit 819a4bd841e0ae8f11941695519d35845c9a8490 Removed the mandatory check. Added functions to allow user application to inspect libuv versions. Added example. Thanks for this suggestion.