[tor-bugs] #2105 [Vidalia]: Pick up the breakpad integration

[Tor Bug Tracker & Wiki]

#2105: Pick up the breakpad integration
-------------------------+--------------------------------------------------
 Reporter:  arma         |          Owner:  chiiph       
     Type:  enhancement  |         Status:  assigned     
 Priority:  major        |      Milestone:  Vidalia-0.3.X
Component:  Vidalia      |        Version:               
 Keywords:               |         Parent:               
   Points:               |   Actualpoints:               
-------------------------+--------------------------------------------------

Comment(by edmanm):

 Replying to [comment:12 chiiph]:
 > With that reasoning, I can argue: what if the crash occurs _in_ the
 crash handler?

 Yes! That's exactly my point! :-) That's exactly why you want to do as
 little as possible in the callback. From the Breakpad design docs:

   * ''Use of the application heap is forbidden. The heap may be corrupt or
 otherwise unusable, and allocators may not function.
   * Resource allocation must be severely limited. The handler may create a
 new file to contain the dump, and it may attempt to launch a process to
 continue handling the crash.''
   * ''Execution on the thread that caused the exception is significantly
 limited. The only code permitted to execute on this thread is the code
 necessary to transition handling to a dedicated preallocated handler
 thread, and the code to return from the exception handler.''
   * ''Handlers shouldn’t handle crashes by attempting to walk stacks
 themselves, as stacks may be in inconsistent states. Dump generation
 should be performed by interfacing with the operating system’s memory
 manager and code module manager.''
   * ''Library code, including runtime library code, must be avoided unless
 it provably meets the above guidelines. For example, this means that the
 STL string class may not be used, because it performs operations that
 attempt to allocate and use heap memory. It also means that many C runtime
 functions must be avoided, particularly on Windows, because of heap
 operations that they may perform.''

 Also:

 ''Authors of both callback functions that execute within a Breakpad
 handler are cautioned that their code will be run at exception time, and
 that as a result, they should observe the same programming practices that
 the Breakpad handler itself adheres to. Notably, if a callback is to be
 used to collect additional data from an application, it should take care
 to read only “safe” data. This might involve accessing only static memory
 locations that are updated periodically during the course of normal
 program execution.''

 It doesn't take much looking at Qt code to see that the changes you've
 made don't conform to these guidelines. And for what? To save writing a
 few lines of code? I could've done that too and saved myself some time,
 but I didn't because it was the lazy, incorrect way to do things.

 > I understand what you mean, but arguing with myself too, because I also
 think I rather have a crash handler for all Vidalia crashes and none of
 the Qt crashes, than having no crash handler in any platform, of having to
 debug string and file operations done "by hand" for buffer overflows and
 such.

 I don't think there's an either/or situation here. Plus, the crash
 handling worked fine on Windows so the statement that there was "no crash
 handler in any platform" is wrong. What was really missing and preventing
 crash dumps from being enabled by default is that someone needs to set up
 a place for the crash dumps to be uploaded, processed and displayed. I had
 set up a Socorro instance awhile ago, but man it was a pain to setup and
 maintain. There also needs to be an easy way for packagers to generate and
 upload symbol files for releases. And, yes, the crash handler callback
 needs to be expanded to work on more platforms than Windows. But writing
 to a file and launching a process are CS101 things that you can do in a
 safe way without relying on Qt and the heap operations it brings in. I
 even left some `#ifdef` statements in there indicating where such code for
 non-Windowses would go.

 > Also, I think that breakpad itself has operations to handle dump
 writing, so that should be great.

 Yep, it already writes the dump itself (thankfully, since that's a
 decidedly non-trivial task).

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2105#comment:13>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online