add read locking through rrd_open() (#800)
* fix cleanup in rrd_open()
* rados
* don't leak rrd_file->rados
* don't mysteriously close STDIN
* !HAVE_MMAP
* don't leak rrd->stat_head
I have no explanation for why we were chosing to leak rrd->stat_head
(and only rrd->stat_head). If it _was_ required for some reason, there was
a bug anyway, as we did not leak it if we fail to allocate live_hdr.
If it's required by legacy ABI, then we should remove rrd_open() from the
public API (and bump the soname). It was originally scheduled for removal
in v1.4.
* avoid unmapping pages that don't belong to us in rrd_close()
When a rados RRD is closed, don't `munmap(0, rrd_file->file_len)` !
Note munmap() does not return an error when there were no pages mapped, so
our callers wouldn't notice even if they check the result of rrd_close().
Also fix the code for munmap errors - so it doesn't format one message and
then immediately replace it with a less specific one. (If both munmap()
and close() fails though, we will describe the later error only).
Also, there's no point rrd_close() setting rrd_file to NULL at the end,
it won't affect the variable used by the caller.
* Add read locking through rrd_open()
Read lock or write lock is taken according to whether RRD_READONLY or
RRD_READWRITE is passed. For librrd compat, the lock is only taken
when explicitly requested by passing RRD_LOCK. Similarly rrd_lock()
is still available, but it should not be needed by new code.
So `RRD_LOCK|RRD_READONLY` is intended to take a read lock, however I must
admit to limitations of this new code:
* On rados, our write lock expires after 2 seconds. This write-lock is
probably useful as a way to detect conflicts. However I would not want
to rely on a lock that works like this to guarantee consistent reads for
a backup. I did not add read locking on rados.
If rados users need to e.g. obtain a consistent backup, they should
create a rados snapshot. (This is able to work because we use a single
atomic rados operation when when we *write* to RRDs).
* On the Windows _locking() function, read locks are treated the same as
write locks.
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/locking
The Windows code required several changes, and has not been compiled
as of this moment.
With Windows _locking(), we should have been unlocking before close().
Apparently the automatic unlock on close() is not guaranteed to occur
synchronously. This isn't specific to _locking(), it's a limitation
of winapi LockFileEx().
https://docs.microsoft.com/cpp/c-runtime-library/reference/locking
https://msdn.microsoft.com/en-us/library/windows/desktop/
aa365203(v=vs.85).aspx
To manually unlock, we need to know what we actually locked.
But on Windows, rrd_lock() was locking the bytes starting at the current
rrd_file->pos, instead of always starting from 0.
Also, I thought RRD_LOCK should lock the file immediately, to guarantee we
don't have conflicting writes. But on Windows we only tried to lock
the bytes that already existed in the file... which would be 0 bytes for
RRD_CREATE. Fortunately, even Windows recognizes this problem and allows
locking bytes that don't exist yet. So we can just lock the maximum number
of bytes allowed. Locking 1 byte would probably work equally well for us,
but using LONG_MAX seems cleaner to me.