ssid, Thank you for creating an issue.

My idea is https://bugs.chromium.org/p/chromium/issues/detail?id=604726#c75

Continues from  crbug.com/604726 

# Comment 77 by erikchen@chromium.org, Today (15 minutes ago)

@hajimehoshi, c#75: That sounds fine, but instead of making an "empty" SharedMemoryHandle, why don't we just give SharedMemory a GUID?

e.g. when we open a SharedMemory region from a SharedMemoryHandle, also save the GUID. :)


# Comment 78 by hajimehoshi@chromium.org, Today (moments ago)

Hmm, I'm afraid I disagree with your idea because

1. Let's assume SharedMemory::GetGUID() would be introduced. Should we need to replace all of SharedMemoryHandle::GetGUID() with this? Of course we know SharedMemory::GetGUID() is required only in DiscardableSharedMemory instead of SharedMemoryHandle::GetGUID(), but this sounds like exposing of implementation details.

2. Simply, SharedMemory and SharedMemoryHandle would have duplicated id information, which sounds not good.

What do you think?

Implemented my idea https://chromium-review.googlesource.com/c/566353/, I feel like we need more discussion though.

Agreed that more discussion can be helpful.

The idea you've proposed violates the basic assumption that two SharedMemoryHandles have the same GUID if and only if they are the same [which was the intended purpose of GUID].

Given that the only consumer that wants to know the GUID of the mapped region [rather than the handle] is SharedMemoryTracker, I recommend the following:

1) Create 
class SharedMemoryTrackerDelegate {
  GUID GetMappedGUID();
}

2) Record the mapped GUID in SharedMemory.

3) Let SharedMemory inherit from SharedMemoryTrackerDelegate.

4) Let SharedMemoryTracker call GetMappedGUID instead of GetHandle()->GetGUID(), which makes the assumption that the handle hasn't been closed.

wdyt? Please ping me when you get online if you want to discuss this further.

I'd would say that the GUID should be a property of the mmaped region rather than a file handle since that makes more sense for a shared memory. Maybe we just need to move GUID from SharedMemoryHandle to SharedMemory. But, I am not sure what the GUID is currently used for.

+Dana may have better ideas.

GUID was introduced to allow identification of SharedMemory regions across processes. It *must* be on SharedMemoryHandle, since that's the IPC-compatible object. 

I still think my suggestion from c#5 is the appropriate way to handle this issue.

> 4) Let SharedMemoryTracker call GetMappedGUID instead of GetHandle()-> GetGUID(), which makes the assumption that the handle hasn't been closed.

So, you meant SharedMemory::GetMappedGUID() returns an ID owned by SharedMemory not SharedMemoryHandle?

BTW, as primiano@ and I have discussed before, the word ID is preferable to the word GUID since ID is simpler. I forgot where we discussed this.

Updated https://chromium-review.googlesource.com/c/566353/. I just added base::SharedMemory::mapped_id representing an ID for a mapped region. This is valid even after Close as long as Unmap is not called.

I didn't use SharedMemoryTrackerDelegate as I didn't understand why this is needed. Please tell me if I am missing something. Thanks.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5d64433a4a096a86544cd6d80f0a18ef066396ce

commit 5d64433a4a096a86544cd6d80f0a18ef066396ce
Author: Hajime Hoshi <hajimehoshi@chromium.org>
Date: Thu Jul 13 14:50:22 2017

Add SharedMemory::mapped_id()

When SharedMemory::Close is called, SharedMemoryHandle is cleared and
losts its ID. This means SharedMemory(Handle)'s ID is not available
after closing.

In DiscardableSharedMemory, shared memory is closed without unmmap-ing
just to save file descriptor resources, and this makes
SharedMemoryTracker confused since SharedmemoryTracker tracks mmap-ed
(and not unmmap-ed) shared memory regions, and tries to get IDs of
those regions. The problem is that sometimes SharedMemoryTarcker fails
to get such IDs.

This CL fixes this problem by adding SharedMemory::mapped_id() to make
the id available even after Close is called.

Bug:  604726 ,  740781 
Change-Id: I5dbf6bd133d8a1e935adad53604f3f40da3c4a6f
Reviewed-on: https://chromium-review.googlesource.com/566353
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486375}
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory.h
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_fuchsia.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_mac.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_nacl.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_posix.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_tracker.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_unittest.cc
[modify] https://crrev.com/5d64433a4a096a86544cd6d80f0a18ef066396ce/base/memory/shared_memory_win.cc