2

Good afternoon, We are trying to build a prototype of Memory Mapped File Caching program for use by Windows and Linux 32 bit applications. Every time we run the prototype we get an Error 487(Error Invalid Address) when we try call UnMapViewOfFile to unmap a a cached memory mapped file region. We think this is happening because we trying a unmap a previouslu unmapped region. We were wondering if it is possible to ignore this error message.

We try our best to make sure that every call to MapViewOfFile is matched by an UnMapViewOfFile in the following way, Every time we call MapViewOfFile , we use the following code: 

std::deque<Range> ranges_type;

std::multimap<char *,Range> mmultimap;

MapPtr = (char*)::MapViewOfFile(hMapping,
                                FILE_MAP_WRITE | FILE_MAP_READ,
                                0, baseoff,
                                mappedlength);
if (MapPtr == 0){
    DWORD lasterr = GetLastError();
    ErrorMessage(lasterr);
}

ranges_type.insert(RangeDeque::value_type(
                       PreviousNCopy,
                       PreviousN,
                       adjustedptr + n,
                       MapPtr,
                       TimeStamp,
                       mappedlength));

mmultimap.insert(RangeMultiMap::value_type(
                     MapPtr,
                     Range(PreviousNCopy,
                           PreviousN,
                           adjustedptr + n,
                           MapPtr,
                           TimeStamp,
                           mappedlength)));

Everytime we unmap a memory mapped file region, we use the following excerpt:

typedef std::multimap<char *,Range>::const_iterator I;
numerased = 0;
std::pair<I,I> b = mmultimap.equal_range(TmpPrevMapPtr);
for (I i=b.first; i != b.second; ++i){ 
    std::dequeue<Range>::iterator iter;
    iter = std::lower_bound(ranges_type.begin(),
                            ranges_type.end(),
                            i->second);
    if (iter != ranges_type.end() && !(i->second < *iter)){
        ranges_type.erase(iter);
        numerased++;
    }
}

erasecount = mmultimap.erase(TmpPrevMapPtr);
retval = UnmapViewOfFile(TmpPrevMapPtr);
if (retval == 0){
    DWORD lasterr = GetLastError();
    ErrorMessage(lasterr);
}

The class Range looks like this:

class Range {
public:
    explicit Range(int item){
        mLow = item;
        mHigh = item;
        mPtr  = 0;
        mMapPtr = 0;
        mStamp = 0;
        mMappedLength = 0;
    }
    Range(int low, int high, char* ptr = 0,char* mapptr = 0, int stamp = 0, int currMappedLength = 0){
        mLow = low;
        mHigh = high;
        mPtr  = ptr;
        mMapPtr = mapptr;
        mStamp = stamp;
        mMappedLength = currMappedLength;
    }

    Range(const Range& r):

    bool operator==(const Range& rhs) const{
        return (mLow <= rhs.mLow && mHigh >= rhs.mHigh);
    }
    bool operator<(const Range& rhs) const{
        return mHigh < rhs.mHigh;      
    } 

public:
    int mLow;   
    int mHigh; 
    char* mPtr;
    char* mMapPtr;
    int mStamp;
    int mMappedLength;
}; // class Range

Thank you for reading this post.

bdonlan
  • 224,562
  • 31
  • 268
  • 324
Frank
  • 1,406
  • 2
  • 16
  • 42
  • The STL Protototypes for our container classes looks like this: std::deque ranges_type; std::multimap mmultimap; where the class Range is defined above, Thank you. – Frank Jun 17 '11 at 17:40
  • We wondering if change the multimap key for char* MapPtr to a struct cache { char* MapPtr, int Marked; }; where we modify the key of the multimap everytime we unmap a cached memory mapped file region? Then, everytime before we call UnMapViewOfFile(TmpPrevMapPtr) we can checked if that if that pointer has alreday been unmapped before. Is that possible to do with the STL multimap class? Thank you. – Frank Jun 17 '11 at 17:47
  • It is certainly possible to ignore this error, it only tells you that the unmap operation didn't work because the address was not mapped. Or, worded differently, what would you want to do to handle the error? There is little you can do (except avoiding it in the first place). – Damon Jun 17 '11 at 17:50
  • @Damon, Thank you for your answer. We agree with you, once the error has happened this is very little we can do to reverse the effects of the mistake we made in our accounting of the MapViewOfFile and UnMapViewOfFile calls. We just wanted to make sure that calling UnMapViewOfFile on an address that was not mapped would not adversely affect MapViewOfFile later. Thank you for your help, – Frank Jun 17 '11 at 17:56
  • Fix the bug instead of trying to find a way to avoid the error message. Bugs that you resolve by circumventing errors only come back to bite you in the behind later (or hide other bugs because the circumvention also affects something else). – Ken White Jun 17 '11 at 18:45
  • @Ken White, Thank you for your answer. We tried changing from 2 STL caches to Single Boost MultiIndex container cache but the STL code seems to run faster than the Boost code even in Windows release. Are we doing something with Boost. We can post our Boost (60 lines) to an URL website and provide you with the link if you are interested. Thank you for your help. – Frank Jun 17 '11 at 19:17

2 Answers2

4

we trying a unmap a previouslu unmapped region

That is a bug, period. You "ignore" the error by fixing the bug.

Alternately, just ignore it with an if test. The Win32 function is telling you there's a bug to fix, but if you want to ignore what it's telling you of course nobody is going to prevent you from doing so.

Billy ONeal
  • 104,103
  • 58
  • 317
  • 552
  • @Billy O"Neal, Thank you for your answer. We agree with you. The bug must be fixed. What do you think if change the multimap key from a char* MapPtr to a struct cache { char* MapPtr, int Marked; }; where we mark/modify the key of the multimap everytime we unmap a cached memory mapped file region? Then, everytime before we call UnMapViewOfFile(TmpPrevMapPtr) we can check if that pointer has already been unmapped before by looking at the Marked integer in the struct key . Is that possible to do with the STL multimap class in Order(Constant) time? Thank you for your answer. – Frank Jun 17 '11 at 18:44
  • @Frank: Why not use something like `shared_ptr` which does all the drudge work for you? You can write your own reference counting but there are plenty of already-written solutions for it. – Billy ONeal Jun 17 '11 at 19:27
  • @Billy ONeal, Thank you for your suggestion. We will look at shared_ptr. Thank you for your help. – Frank Jun 17 '11 at 19:32
  • @Billy ONeal, I just accepted your answer. Thank you for your help. – Frank Jun 18 '11 at 23:44
  • @Billy ONeal, We experimented with your Boost shared_ptr idea. It turns Boost makes it very hard for someone to convert or assign a raw C++ pointer to a Boost shared_ptr object. Could you please tell us where to find out how to convert or assign a WINAPI(i.e.MapViewOfFile) raw pointer to a Boost shared_ptr object. Thank you for your help, – Frank Jun 20 '11 at 18:46
  • @Frank: `shared_ptr::reset`. – Billy ONeal Jun 20 '11 at 19:26
  • @Billy ONeal, Boost::shared_ptr::reset() drops the reference count by 1. So, do we call shared_ptr::reset before calling UnMapViewOfFile(shared_ptr::get()). We are also assigning the MapViewOfFile(...) return value to a shared pointer using the following statement:boost::shared_ptr FileMappingRegion; FileMappingRegion = boost::shared_ptr( (char*)::MapViewOfFile(hMapping, FILE_MAP_WRITE | FILE_MAP_READ, 0, baseoff,mappedlength)); Is that the correct way? Thank you for your reply. – Frank Jun 20 '11 at 21:18
  • @Frank: I'm confused -- was assuming you'd just create a class for the mapping, then maintain handles to the class as `shared_ptr`s. Having to call UnmapViewOfFile manually was the whole thing to avoid in the first place. :) – Billy ONeal Jun 20 '11 at 21:34
  • @Billy ONeal,We considered created a class for the memory mapping:class MapPtr: public enable_shared_from_this where the class MapPtr would handle the call to UnmapViewOfFile and MapViewOfFile. Thank you for your reply. – Frank Jun 20 '11 at 21:57
1

You need to fix the underlying bug that causes the memory region to be unmapped twice. After all consider this:

  • You map a region of memory (call it A) at address 0x42000000
  • You unmap A at 0x42000000
  • You map a region of memory B at the same address 0x42000000
  • You double-unmap A at 0x42000000 - only this time B is unmapped
  • You try to access B, and crash.

You need to figure out where you're double-unmapping this memory and fix it, or something like this can happen eventually. If you hide the error, it'll just make this that much more confusing to debug.

Now, your accounting is fine for debug purposes, but it's not fixing the root cause; you're not keeping good track of your memory mappings in the first place. It's not possible to comment much more on the small portion of code you've posted, but you should be looking at the code that decides when to map/unmap as what you need to fix; don't just try to suppress the double-frees when it's too late. After all, if you're double-freeing this memory mapping, does this mean your code thinks it's still mapped? In which case what's stopping access-after-free problems from occurring:

  • Map A at 0x42000000
  • Unmap A at 0x42000000
  • Access A (crash!)
  • Try to double-unmap A at 0x42000000 (not reached, due to the crash)

Or:

  • You map a region of memory (call it A) at address 0x42000000
  • You unmap A at 0x42000000
  • You map a region of memory B at the same address 0x42000000
  • Thinking A is still mapped, you access memory at 0x42000000 and access B instead. Confusion! Or possibly data corruption!
bdonlan
  • 224,562
  • 31
  • 268
  • 324
  • @BDonlan, Thank you for your thoughtful answer. We have two STL caches in our prototype, std::deque ranges_type; std::multimap mmultimap. Everytime we try unmap a region of memory, we erase items from the ranges_types cache and the mmultimap cache. In order to access a region of memory X, either it must be in the ranges_type cache or we have to unmap the least recently used region of memory and then call MapViewOfFile to map another region of memory. Theoretically, it is impossible to access X after it has been unmapped. Thank you. – Frank Jun 17 '11 at 19:04
  • @Frank, if it's impossible to access X, why is it possible to unmap X after it has been unmapped? Therein lies your bug :) – bdonlan Jun 17 '11 at 19:05
  • @BDonlan, I apologize I still have more questions. We agree with our 2 STL cache system is not bug-free. Should we change to single BOOST MultiIndex caches with multiple indices? We tried doing that but STL seems to run faster than the BOOST multi-index code even in the Windows release version? Perhaps , we aren't using the BOOST container class correctly. We can post our code to URL system and provide you with a URL link if you wish. Thank you for your thoughtful answer. – Frank Jun 17 '11 at 19:11
  • @Frank, your problems are more fundamental than usage of STL/boost containers. You're not keeping good track of your allocations, simple as that - this is a classic double-free issue. While tracking with multimaps or whatever may help you debug, _it is not a fix_. – bdonlan Jun 17 '11 at 19:15
  • @BDonlan, We agree with you. There is a bug in our 2 STL cache system. We will try to switch to a single Boost cache to better keep track of our allocations. It is just that STL code seems to run faster. Maybe we should start another post about our Boost code after we test it again. Thank you for your help. – Frank Jun 17 '11 at 19:23
  • @BDonlan, Thank you for your help. Here are two www.pastebin.coms URLS with our memory mapping file caching prototype code. Please look at them if you have the time. We would be interested if you notice any allocation tracking problems in this code. Thank you. . Please let us know if you know you can retrieve them. Otherwise, we could email them to you. Thank you. – Frank Jun 18 '11 at 01:45
  • @Frank, while I'm happy to answer questions that don't take too long on SO, fully reviewing someone else's code for what is, presumably, some proprietary application (do you have permission to post that code from your employer?) for free is not something I usually do, sorry. You could try posting another question with those links to http://codereview.stackexchange.com/ to see if someone has the time to review it, though. – bdonlan Jun 18 '11 at 20:09
  • @BDonlan, I understand your concern. I took out all the proprietary code from the www.pastebin.com url link before I posted it. I just accepted your answer. Thank you. – Frank Jun 18 '11 at 23:42