(No comment was entered for this change.)

I'm not really sure what's going on here.  It seems pretty bizarre for IOBuffer::data_ 
to be NULL here.  I'll sprinkle some CHECKs and see if anything turns up.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=20415 

------------------------------------------------------------------------
r20415 | willchan@chromium.org | 2009-07-10 14:03:17 -0700 (Fri, 10 Jul 2009) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/http/http_network_transaction.cc?r1=20415&r2=20414
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/socket/ssl_client_socket_win.cc?r1=20415&r2=20414

Add some CHECKs to track down the source of a NULL deref in the SSLClientSocketWin code.
BUG=http://crbug.com/16371
TEST=none

Review URL: http://codereview.chromium.org/155359
------------------------------------------------------------------------

This is not a release blocker. It's in the long tail of crashes.

from 195.1:

http://crash/reportdetail?
reportid=0f4a5c8c9e6103c3&product=Chrome&version=3.0.195.1&date=&signature=logging::LogMessage::~LogMessage()-1B5BBBF

[logging.cc:553]	 logging::LogMessage::~LogMessage()
[http_network_transaction.cc:361]	 net::HttpNetworkTransaction::Read(net::IOBuffer *,int,CallbackRunner<Tuple1<int> > 
*)
[http_cache.cc:955]	 net::HttpCache::Transaction::ReadFromNetwork(net::IOBuffer *,int)
[http_cache.cc:507]	 net::HttpCache::Transaction::Read(net::IOBuffer *,int,CallbackRunner<Tuple1<int> > *)
[url_request_http_job.cc:439]	 URLRequestHttpJob::ReadRawData(net::IOBuffer *,int,int *)
[url_request_job.cc:137]	 URLRequestJob::Read(net::IOBuffer *,int,int *)
[url_request.cc:335]	 URLRequest::Read(net::IOBuffer *,int,int *)
[resource_dispatcher_host.cc:1228]	 ResourceDispatcherHost::Read(URLRequest *,int *)
[resource_dispatcher_host.cc:982]	 ResourceDispatcherHost::OnResponseStarted(URLRequest *)
[url_request.cc:352]	 URLRequest::ResponseStarted()
[url_request_job.cc:379]	 URLRequestJob::NotifyHeadersComplete()
[url_request_http_job.cc:571]	 URLRequestHttpJob::NotifyHeadersComplete()
[url_request_http_job.cc:469]	 URLRequestHttpJob::OnStartCompleted(int)
[task.h:578]	 CallbackImpl<`anonymous namespace'::RecentlyBookmarkedHandler,void ( 
A0xeafce67f::RecentlyBookmarkedHandler::*)(Value const *),Tuple1<Value const *> >::RunWithParams(Tuple1<Value const *> const 
&)
[http_cache.cc:633]	 net::HttpCache::Transaction::HandleResult(int)
[http_cache.cc:1167]	 net::HttpCache::Transaction::OnNetworkInfoAvailable(int)
[task.h:578]	 CallbackImpl<`anonymous namespace'::RecentlyBookmarkedHandler,void ( 
A0xeafce67f::RecentlyBookmarkedHandler::*)(Value const *),Tuple1<Value const *> >::RunWithParams(Tuple1<Value const *> const 
&)
[http_network_transaction.cc:428]	 net::HttpNetworkTransaction::OnIOComplete(int)
[task.h:578]	 CallbackImpl<ExtensionsDOMHandler,void ( ExtensionsDOMHandler::*)(Value const *),Tuple1<Value const *> 
>::RunWithParams(Tuple1<Value const *> const &)
[tcp_client_socket_win.cc:501]	 net::TCPClientSocketWin::DoReadCallback(int)
[tcp_client_socket_win.cc:562]	 net::TCPClientSocketWin::DidCompleteRead()
[tcp_client_socket_win.cc:194]	 net::TCPClientSocketWin::Core::ReadDelegate::OnObjectSignaled(void *)
[object_watcher.cc:30]	 base::ObjectWatcher::Watch::Run()
[message_loop.cc:313]	 MessageLoop::RunTask(Task *)
[message_loop.cc:321]	 MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &)
[message_loop.cc:427]	 MessageLoop::DoWork()

Crashing on the second check:

  CHECK(buf);
  CHECK(buf->data());  <----

The buffer comes directly from RDH:

bool ResourceDispatcherHost::Read(URLRequest* request, int* bytes_read) {
...
  net::IOBuffer* buf;
  int buf_size;
  if (!info->resource_handler->OnWillRead(info->request_id,
                                          &buf, &buf_size, -1)) {
    return false;
  }
  DCHECK(buf);
  DCHECK(buf_size > 0);
  info->has_started_reading = true;
  return request->Read(buf, buf_size, bytes_read);

But it's hard to know which handler is creating the buffer. We should probably add another check here.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=21940 

------------------------------------------------------------------------
r21940 | willchan@chromium.org | 2009-07-28 20:42:00 -0700 (Tue, 28 Jul 2009) | 4 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/async_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/async_resource_handler.h?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/buffered_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/cross_site_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/download_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/download_throttling_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/safe_browsing_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/save_file_resource_handler.cc?r1=21940&r2=21939
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/sync_resource_handler.cc?r1=21940&r2=21939

Add CHECKs to the ResourceHandler derived classes to see which is returning a NULL IOBuffer:data_.
BUG=http://crbug.com/16371

Review URL: http://codereview.chromium.org/159561
------------------------------------------------------------------------

A report of somebody hitting one of the CHECKS added in r21940 to track this bug was 
posted as a comment on  bug 17363 . They hit it on Linux.

Awesome, thanks for the info.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=22214 

------------------------------------------------------------------------
r22214 | willchan@chromium.org | 2009-07-31 16:01:20 -0700 (Fri, 31 Jul 2009) | 4 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/async_resource_handler.cc?r1=22214&r2=22213

CHECK that IOBuffer::data_ in AsyncResourceHandler is not NULL before caching it.
BUG=http://crbug.com/16371

Review URL: http://codereview.chromium.org/159730
------------------------------------------------------------------------

From a crash report:

0x01ff649b	 [chrome.dll	 - logging.cc:553]	 
logging::LogMessage::~LogMessage()
0x020b4962	 [chrome.dll	 - async_resource_handler.cc:92]	 
AsyncResourceHandler::OnWillRead(int,net::IOBuffer * *,int *,int)
0x020bddae	 [chrome.dll	 - safe_browsing_resource_handler.cc:108]	 
SafeBrowsingResourceHandler::OnWillRead(int,net::IOBuffer * *,int *,int)
0x020bd388	 [chrome.dll	 - buffered_resource_handler.cc:100]	 
BufferedResourceHandler::OnWillRead(int,net::IOBuffer * *,int *,int)
0x0205478b	 [chrome.dll	 - resource_dispatcher_host.cc:1285]	 
ResourceDispatcherHost::Read(URLRequest *,int *)
0x02054163	 [chrome.dll	 - resource_dispatcher_host.cc:1037]	 
ResourceDispatcherHost::OnResponseStarted(URLRequest *)
0x02219b0f	 [chrome.dll	 - url_request.cc:354]	 
URLRequest::ResponseStarted()
0x022175b6	 [chrome.dll	 - url_request_job.cc:400]	 
URLRequestJob::NotifyHeadersComplete()
0x0223d863	 [chrome.dll	 - url_request_http_job.cc:570]	 
URLRequestHttpJob::NotifyHeadersComplete()
0x0223d553	 [chrome.dll	 - url_request_http_job.cc:468]	 
URLRequestHttpJob::OnStartCompleted(int)
0x022037ee	 [chrome.dll	 - task.h:307]	 RunnableMethod<SSLErrorHandler,void 
( SSLErrorHandler::*)(int),Tuple1<net::Error> >::Run()
0x01ffe499	 [chrome.dll	 - message_loop.cc:313]	 MessageLoop::RunTask(Task *)
0x01ffe4d2	 [chrome.dll	 - message_loop.cc:321]	 
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const &)
0x01ffe661	 [chrome.dll	 - message_loop.cc:428]	 MessageLoop::DoWork()
0x0200ec33	 [chrome.dll	 - message_pump_win.cc:469]	 
base::MessagePumpForIO::DoRunLoop()
0x0200e737	 [chrome.dll	 - message_pump_win.cc:52]	 
base::MessagePumpWin::RunWithDispatcher(base::MessagePump::Delegate 
*,base::MessagePumpWin::Dispatcher *)
0x0200e57c	 [chrome.dll	 - message_pump_win.h:78]	 
base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x01ffe34e	 [chrome.dll	 - message_loop.cc:198]	 MessageLoop::RunInternal()
0x01ffe31d	 [chrome.dll	 - message_loop.cc:181]	 MessageLoop::RunHandler()
0x01ffe2c0	 [chrome.dll	 - message_loop.cc:155]	 MessageLoop::Run()
0x02009065	 [chrome.dll	 - thread.cc:156]	 base::Thread::ThreadMain()
0x01fff7d2	 [chrome.dll	 - platform_thread_win.cc:26]	 `anonymous 
namespace'::ThreadFunc(void *)
0x7c80b728	 [kernel32.dll	 + 0x0000b728]	 BaseThreadStart

It looks like when we construct SharedIOBuffer the shared_memory_ does not always get 
created/mapped properly.  Here's the code:

bool AsyncResourceHandler::OnWillRead(int request_id, net::IOBuffer** buf,
                                      int* buf_size, int min_size) {
  DCHECK(min_size == -1);
  static const int kReadBufSize = 32768;
  if (g_spare_read_buffer) {
    DCHECK(!read_buffer_);
    read_buffer_.swap(&g_spare_read_buffer);
    // TODO(willchan): Remove after debugging  bug 16371 .
    CHECK(read_buffer_->data());
  } else {
    read_buffer_ = new SharedIOBuffer(kReadBufSize);
    if (!read_buffer_->ok())
      return false;
    // TODO(willchan): Remove after debugging  bug 16371 .
    CHECK(read_buffer_->data());  <====CRASHES HERE
  }
  *buf = read_buffer_.get();
  *buf_size = kReadBufSize;
  return true;
}

Here's the definition of SharedIOBuffer:

// Our version of IOBuffer that uses shared memory.
class SharedIOBuffer : public net::IOBuffer {
 public:
  SharedIOBuffer(int buffer_size) : net::IOBuffer(), ok_(false) {
    if (shared_memory_.Create(std::wstring(), false, false, buffer_size) &&
        shared_memory_.Map(buffer_size)) {
      ok_ = true;
      data_ = reinterpret_cast<char*>(shared_memory_.memory());
    }
  }
  ~SharedIOBuffer() {
    // TODO(willchan): Remove after debugging  bug 16371 .
    CHECK(g_spare_read_buffer != this);
    data_ = NULL;
  }

  base::SharedMemory* shared_memory() { return &shared_memory_; }
  bool ok() { return ok_; }

 private:
  base::SharedMemory shared_memory_;
  bool ok_;
};

As of the Linux crash, it is fixed by forcing /dev/shm in /etc/fstab. I have used 2G
& it works but I do not know which value would be the most suited.

Yeah, I got an internal report today from a Googler with an issue with /dev/shm.  His 
/dev/shm didn't have the correct permissions, so Chrome would fail to create temporary 
shared memory files there.

 Issue 19208  has been merged into this issue.

Shouldn't something be logged if it fails to create temporary files in /dev/shm?  I
just ran into this same problem since I was running in a schroot without /dev/shm
setup properly and it took me several days to find this since the error messages
aren't helpful.  Failing to write to shm seems like a pretty big problem for a multi
process application to not report.

 Issue 19240  has been merged into this issue.

 Issue 20591  has been merged into this issue.

 Issue 20999  has been merged into this issue.

 Issue 21244  has been merged into this issue.

And another internal report, from e.gluzberg.

What the heck, this seems common enough, at least on Linux; let's bump it up in
priority.  Or should we have a separate bug for the /dev/shm permissions problem?

Hm, I'm not really sure how to handle this.  If we can't create shared memory, that's a 
problem and we should just crash. I'm not sure why people's perms are set incorrectly 
for /dev/shm so often.  Maybe we need to crash with a fatal popup message.  I guess 
we could switch to shmget() instead if we can figure out what to do for StatsTable which 
seems to need a named file (I'm not sure why).  I'm taking off for vacation tomorrow, so 
I'm unassigning this.

 Issue 21512  has been merged into this issue.

(No comment was entered for this change.)

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=26361 

------------------------------------------------------------------------
r26361 | evan@chromium.org | 2009-09-16 10:32:11 -0700 (Wed, 16 Sep 2009) | 10 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/shared_memory.h?r1=26361&r2=26360
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/shared_memory_posix.cc?r1=26361&r2=26360

posix: clean up shared memory code

At first I rewrote this to use the shm_*() family of
functions, but that doesn't work on OS X.  Now I've just
cleaned up some bits and added an extra print to help with
the below bug.

BUG=16371

Review URL: http://codereview.chromium.org/204024
------------------------------------------------------------------------

FYI: The DCHECK added to investigate this bug is the #6 browser topcrash in 4.0.207.0 
on OS X.

http://crash/reportdetail?reportid=d5579759089d33f6

IMHO this should be marked as a beta blocker.

(No comment was entered for this change.)

The commit I did in comment 23 is the extent of what I intended to do.

This crash, at least on Linux, is caused by being unable to set up a shared memory 
object.  At least on Linux, the extent of this bug is when people have /dev/shm set up 
improperly we hoark.  "/dev/shm set up improperly" is total jargon and normal users 
have it set up right for themselves.

I added a LOG(ERROR) to see if that flushes out any bug reports.

On OS X, the temporary memory space is backed by a file in /tmp (which seems dangerous 
-- what if the kernel decides to flush it? what if you run out of disk space?).  
Perhaps adding more LOG(ERROR) (or even CHECK()) near the ftruncate call would help 
flush those out.

Oh, also: I rewrote all this code to use shm_open() in the thought that might help 
with our /dev/shm problems, but I discovered:
 - on Linux, glibc's shm_open() just does what we're currently doing (opening a file 
in /dev/shm)
 - on Mac, it's limited to 4mb.  I added a comment mentioning this so nobody else 
wastes a day on this.

Had the same issue.  For what it is worth, occasionally I do something to bork
permissions in /dev.  Haven't been able to fix it without a complete reboot no matter
how hard I try.  This is on Ubuntu 8.10, running awesome3 window manager.

Looks like I hit the same thing, but "chmod 777 /dev/shm" fixed it (perhaps 777 is
excessive)

This is the #9 topcrash on OS X 4.0.22.1 and it doesn't seem to have an owner.

Flipping to untriaged so one gets assigned.

This crash was found in 4.0.220.1 and is currently ranked #18 (based on the relative number of reports in the release).  There have been 19 reports from 15 clients.

Report Link: http://crash/reportdetail?reportid=24305bf060ecb598
http://crash/search?query=Chrome+4.0.220.1+AsyncResourceHandler%3A%3AOnResponseCompleted%28int%2C+URLRequestStatus+const%26%2C+std%3A%3Astring+const%26%29

 Issue 23965  has been merged into this issue.

(No comment was entered for this change.)

From the current crashes, and based on comment #20, I think that someone familiar with 
OS X internals should see if there is a configuration problem that we can fix somehow 
to avoid failing to create the shared memory.

Moving to untriaged as Jeremy will be in transit for awhile.

(No comment was entered for this change.)

> On OS X, the temporary memory space is backed by a file in /tmp
> (which seems dangerous  -- what if the kernel decides to flush
> it? what if you run out of disk space?

To that I'll add: what if the user is running off a read-only system volume? Mac OS X
also has a potential directory permissions problem, since the user may not have write
permissions on the temp directory (unlikely perhaps, but no less likely than
incorrect permissions on /dev/shm).

That said, for Mac OS X I'm leaning toward the culprit being ftruncate() failing on
an out-of-disk-space condition. With permissions problems, we should fail immediately
at launch, since we create a bunch of shared memory from the get-go. The crash logs,
however, show uptimes like 7 seconds, 58 seconds, 86 seconds, and so on. That tracks,
since we don't allocate large chunks of shared memory (multiple MBs) until the user
starts browsing.

I'm adding a CHECK() to the ftruncate() call to confirm.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=30530 

------------------------------------------------------------------------
r30530 | hawk@chromium.org | 2009-10-29 16:11:00 -0700 (Thu, 29 Oct 2009) | 5 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/shared_memory_posix.cc?r1=30530&r2=30529

Add a CHECK() around the SharedMemory ftruncate() call to finger it as a crash culprit.

BUG=16371
TEST=Check the crash logs to see if this CHECK() supplants the CHECK() failure in AsyncResourceHandler::OnWillRead()
Review URL: http://codereview.chromium.org/347012
------------------------------------------------------------------------

hawk: Crashes are here:
http://crash/reportdetail?reportid=f1ccf3778371cf25

willchan: can we remove the CHECK @ async_resource_handler.cc:178 ?
http://crash/reportdetail?reportid=458f64c2403acbf5#crashing_thread

I don't think so.  The CHECK should be valid.  My memory's fuzzy, but IIRC, we're 
caching an IOBuffer which does not have a valid data pointer.  The next time we try to 
reuse this IOBuffer, we'll crash.  Do you have a reason to expect that this is not the 
case?

hawk, any progress with this beta blocker?

(No comment was entered for this change.)

Not sure why this got assigned to me.  Hawk's got a changelist for this.  Bouncing 
back to hawk.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=32701 

------------------------------------------------------------------------
r32701 | hawk@chromium.org | 2009-11-20 15:08:38 -0800 (Fri, 20 Nov 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/shared_memory_posix.cc?r1=32701&r2=32700
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/async_resource_handler.cc?r1=32701&r2=32700

Don't do work in the SharedIOBuffer constructor; use an Init method instead. This eliminates instances of allocated-but-invalid SharedIOBuffers. Also clean up some CHECKs and TODOs associated with the bug.

The upshot is that we no longer crash in AsyncResourceHandler::OnResponseCompleted() when we can't allocate shared memory. We now crash (properly, I believe) in the renderer process if the shared memory that failed to allocate was the TransportDIB, since the renderer can't communicate with the browser without it.

BUG=16371
TEST=none 
Review URL: http://codereview.chromium.org/391009
------------------------------------------------------------------------

Should this be landed on the branch?

Yes, we should. See also  issue 28608 .

 Issue 28608  has been merged into this issue.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=33126 

------------------------------------------------------------------------
r33126 | mal@chromium.org | 2009-11-25 13:38:54 -0800 (Wed, 25 Nov 2009) | 12 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/249/src/base/shared_memory_posix.cc?r1=33126&r2=33125
   M http://src.chromium.org/viewvc/chrome/branches/249/src/chrome/browser/renderer_host/async_resource_handler.cc?r1=33126&r2=33125

Merge 32701 - Don't do work in the SharedIOBuffer constructor; 

use an Init method instead. This eliminates instances of allocatedbutinvalid SharedIOBuffers. Also clean up some CHECKs and TODOs associated with the bug.

The upshot is that we no longer crash in AsyncResourceHandler::OnResponseCompleted() when we can't allocate shared memory. We now crash (properly, I believe) in the renderer process if the shared memory that failed to allocate was the TransportDIB, since the renderer can't communicate with the browser without it.

BUG=16371
TEST=none 
Review URL: http://codereview.chromium.org/391009

TBR=hawk@chromium.org
Review URL: http://codereview.chromium.org/437070
------------------------------------------------------------------------

Presumed fixed... re-open if I got that wrong.

This bug was closed without addressing any of the CHECKs added in r21940, 
http://codereview.chromium.org/159561.  A lot of those have comments to the effect 
of "remove after debugging  bug 16371 ."

Will, should we remove those CHECKs now?  Should we file a follow-up bug to do it?  Or 
do we want to keep those CHECKs and just get rid of the comments referring to this 
bug?  Or something else?

Hawk, correct me if I'm wrong, but as I recall, your fix only addresses the issue for 
Mac, not for Linux.  Linux users still have a problem if they fail to create shared 
memory, which seems to be happening for users with incorrect perms for /dev/shm.  
This CHECK should not be removed for them because they will just crash later when 
they try to access the shared memory.  We should probably fix this by failing to 
startup and displaying an error message telling the user how to fix their perms, 
since we can't do anything without the shared memory anyway.

I think we should file a followup bug to do this.

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=32701 

------------------------------------------------------------------------
r32701 | hawk@chromium.org | 2009-11-20 15:08:38 -0800 (Fri, 20 Nov 2009) | 7 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/shared_memory_posix.cc?r1=32701&r2=32700
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/renderer_host/async_resource_handler.cc?r1=32701&r2=32700

Don't do work in the SharedIOBuffer constructor; use an Init method instead. This eliminates instances of allocated-but-invalid SharedIOBuffers. Also clean up some CHECKs and TODOs associated with the bug.

The upshot is that we no longer crash in AsyncResourceHandler::OnResponseCompleted() when we can't allocate shared memory. We now crash (properly, I believe) in the renderer process if the shared memory that failed to allocate was the TransportDIB, since the renderer can't communicate with the browser without it.

BUG=16371
TEST=none 
Review URL: http://codereview.chromium.org/391009
------------------------------------------------------------------------

Don't see this with 4.0.249.22

(No comment was entered for this change.)