Fix mmap.resize errors

[BCSharp]

No description provided.

[slozier]

Thanks!

[BCSharp]

I wanted to do more but I realized that I do not understand what the goal is here. Normally, when there is an problem with the code it is kind of evident what the original intention of it was. But here, is it a bug or a feature? Is the interface of mmap supposed to be thread-safe or not?

If yes, then how come all the effort is in making close thread-safe with concurrent execution of other regular methods (it wasn't, and it still is not), but not the regular methods between themselves? (MmapLocker is not locking anything.) Especially resize vs. other methods? I've never seen a pattern like that, when closing is (supposed to be) safe concurrently with the regular usage, but the regular usage is not thread-safe. It is normally the opposite: all methods are thread-safe, except closing/disposing, which has to happen after all usage ceases.

If not, why all this heavy lifting by locks, interlocked inc/dec, reference counting etc. If the interface is need not be thread-safe, all this overhead can go and simplify things considerably.

@slozier, do you know or suspect what the requirements are for the mmap interface? And is the .NET MemoryMappedFile/MemoryMappedViewAccessor thread-safe?

Right now what we have is a not-thread-safe interface with a lot of extra complexity that does not provide anything useful.

[slozier]

do you know or suspect what the requirements are for the mmap interface? And is the .NET MemoryMappedFile/MemoryMappedViewAccessor thread-safe?

I have to admit I haven't a clue why the MmapLocker exists. Other than doing the EnsureOpen check (which I guess we'd have to do everywhere anyway), the _refCount increment/decrement does not seem to serve a purpose. I guess it ensures some sort of consistent closed state? As for the mmap module itself I could imagine wanting to use multiple threads with it. However I would argue that if a program is doing stuff to alter the internal state (resize and close) while doing multi-threaded use of the same mmap then I don't think that's our problem? I don't know the thread-safety of the .NET classes, but at a glance it seems like having multiple threads using the same accessor would be fine.

[BCSharp]

Thanks for your insights. So my best guess now is that perhaps mmap was work in progress towards thread-safety that was abandoned half-way. Given this assumption and that having thread-safety is desirable, I think the best way moving forward is to embrace MmapLocker and at least make close thread-safe and indeed guaranteeing a consistent closed state. It should be simple (well, relatively speaking) esp. in comparison to resize, for two main reasons:

1. close changes the state of mmap irreversibly, so incoming calls on other threads do not need to wait. They will be either let in or thrown out with an exception (ODE). OTOH resize will require some sort of locking, because after resize ends, mmap becomes usable again.
2. close has no parameters. The only information it conveys is that it happened, which means literally just one bit of information. It will be easy to combine it with other data like reference counting in an atomic operation, without resorting to expensive locking.

Also, the difference between close and resize is that resize is discretionary — so we can expect the user code to take some precautions, or at least take it into consideration. OTOH close is, if not mandatory, then highly expected to be performed for each mmap created, so anything that guarantees a consistent closed state is a win.

I will come up with a solution for close in one of the next PRs. I do appreciate the efforts in MmapLocker to avoid locking as much as possible, especially in regular use cases. This is possible for close but will not be possible for resize.