libreoffice-online/PROBLEMS

- The URL of the document to view and/or edit is specified both in the
  HTTP GET and in the 'load' message. This is redundant, and adds
  completely unnecessary, hard to define, and hard to test, states
  that the server and clients can be in. For instance:

  - Is it an error to send a different URL in the 'load' message than
    the one in the GET? (Of course it is. Do we check that? Do we have
    a unit test to make sure it stays an error?)

  - What messages are accepted by the server before the 'load'? What
    messages are sent to the client? Do they make sense, if it is only
    the 'load' that actually fully completes the association of a
    session with a document? Do we have unit tests for this?

  - What if one client connects, and then before it has sent the
    'load' message, another client connects specifying the same
    document in the GET, and also sends a 'load' message. Which client
    gets the edit lock? Is that arbitrary or specified? Do we have a
    unit test for it?

  - Etc

- There is way too much of busy waiting for fairly arbitrarily chosen
  timeout periods in the code.

  With "busy wait" here I of course don't mean *real* busy wait that
  would actually mindlessly spin the CPU.

  I mean calling poll() with some short timeout (order of seconds),
  and each time a timeout happens (which surely is the large majority
  of times), check a condition, which in the large majority of times
  will not be true, and then repeat.

  Instead the code could use for instance eventfds to enable the
  poll() calls to be without timeout. Or something similar, depending
  on case.

- Recursive mutexes are evil. In general, I think the consensus is
  that recursive mutexes should be avoided. One should use them only
  when absolutely necessary because the code-base is so complex that
  one has no idea how it works. That was hopefully not the case when
  recursive mutexes were introduced here? But probably it is by now...

- Occasionally Control-C (SIGINT) doesn't shut down loolwsd. One has
  to kill it with SIGKILL. Which of course leaves all the chroot jails
  around.

- There are lots of places where a std::string variable is defined,
  initialised with a value, that is never changed. (In many cases it
  is const, so could of course not be changed.) Then the variable is
  used just once or twice, passed as a parameter to a function, or
  used in a comparison expression. This is fairly pointless and just
  makes the code harder to read. Use string literals instead.