| View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
| ID | Project | Category | View Status | Date Submitted | Last Update | ||||
| 0008554 | VTK | (No Category) | public | 2009-02-17 17:47 | 2009-08-13 16:11 | ||||
| Reporter | Sean McBride | ||||||||
| Assigned To | François Bertel | ||||||||
| Priority | normal | Severity | crash | Reproducibility | always | ||||
| Status | closed | Resolution | fixed | ||||||
| Platform | Mac OS X | OS | Mac OS X | OS Version | 10.5 | ||||
| Product Version | |||||||||
| Target Version | Fixed in Version | ||||||||
| Summary | 0008554: vtkOpenGLExtensionManager::ReadOpenGLExtensions derefs NULL on headless Macs | ||||||||
| Description | vtkOpenGLExtensionManager::ReadOpenGLExtensions passes NULL to sscanf() on a Mac that has no connected monitor. This causes a crash. Patch attached. | ||||||||
| Tags | No tags attached. | ||||||||
| Project | |||||||||
| Type | |||||||||
| Attached Files | |||||||||
| Relationships | |
| Relationships |
| Notes | |
|
(0015149) François Bertel (developer) 2009-02-18 13:17 |
Can you clarify what a headless Mac is? 1. Is is a Mac with a graphics card but with no connected monitor to it or 2. is it a Mac with no graphics card at all? In any case, can you really create a window on it (I mean from the carbon or cocoa, out of VTK)? Because what's the screen size in these cases? Do you have a reference on the Mac documentation for this behavior? |
|
(0015150) Sean McBride (developer) 2009-02-18 13:24 edited on: 2009-02-18 13:33 |
I refer to (1). I haven't ever tested (2). If no monitor is attached, one can still connect using "Screen Sharing" (a feature added in Mac OS X 10.5, basically a synonym for VNC). When using the computer in this way, you can still run GUI applications as usual. I admit it's a pathological case. But the simple patch attached stops my app from crashing when used in this way. I don't see any VTK rendering, which is fine. But crashing is bad. :) |
|
(0015354) Sean McBride (developer) 2009-02-24 17:50 |
François, did you have a chance to look at the patch? It would be great to get this committed before the 5.4 branch. |
|
(0015463) François Bertel (developer) 2009-02-27 16:15 |
Which concrete class of vtkOpenGLRenderWindow do you have in this case? X11? Carbon? Cocoa? OSMesa? If you apply your patch and it does not crash, can you still see something displayed in the RenderWindow? If not, my guess is the creation of the OpenGL context fails inside the concrete class of vtkOpenGLRenderWindow. Unfortunately, there is no API on vtkOpenGLRenderWindow that reports this status. However, if the OpenGL context creation fails, then vtkOpenGLRenderWindow::IsCurrent() should return false. But this is not the only reason why IsCurrent() can return false. Can you try to add cout << "IsCurrent=" << this->RenderWindow->IsCurrent() << endl; before line 482-482 which has glGetString(GL_VERSION); glGetString() is an OpenGL call and it's only valid to call it if there is a OpenGL context current on the running thread. For more investigation, you can put some code in the concrete vtkOpenGLRenderWindow: * If you are using Cocoa, try to display some information in void vtkCocoaRenderWindow::CreateGLContext() after the line creating the NSOpenGLContext. NSOpenGLContext* context = [[[NSOpenGLContext alloc] initWithFormat:pixelFormat shareContext:nil] autorelease]; * If you are using Carbon, void vtkCarbonRenderWindow::CreateAWindow() seems to already display some error if the context creation fails: this->ContextId = this->Internal->CreateContext(0, this->DoubleBuffer, this->StereoCapableWindow, this->MultiSamples, this->AlphaBitPlanes, this->StencilCapable, error); if (NULL == this->ContextId) { if(error) { vtkErrorMacro(<<error); } return; } * If you are using X11, void vtkXOpenGLRenderWindow::CreateAWindow() already has some message if the context creation fails: if(!this->Internal->ContextId) { vtkErrorMacro("Cannot create GLX context. Aborting."); * If you use OSMesa, in void vtkOSOpenGLRenderWindow::CreateOffScreenWindow(int width, int height) try to display something after: this->Internal->OffScreenContextId = OSMesaCreateContext(GL_RGBA, NULL); By the way, by reviewing vtkOpenGLExtensionManager.cxx, I found a flaw in ReadOpenGLExtensions() where there are a few other places using gl*() glu*() functions that may happen while there is no OpenGL context created and current yet. I agree with the comment of Jean-Francois Roy on the question you posted on the Apple mailing-list: http://lists.apple.com/archives/mac-opengl/2008/Sep/msg00080.html [^] |
|
(0015466) Sean McBride (developer) 2009-02-27 16:44 |
To answer your questions: - I only use Cocoa - With my patch I don't crash, but I don't see anything in the renderwindow (which, for my purposes, is acceptably sufficient). - I will try the other things on Monday... |
|
(0017091) Sean McBride (developer) 2009-08-11 10:34 |
$ cvs commit -m "BUG: superficial fix of bug 8554: test pointer for null before dereferencing it. Root cause still needs investigation." vtkOpenGLExtensionManager.cxx new revision: 1.36; previous revision: 1.35 |
|
(0017092) François Bertel (developer) 2009-08-11 10:52 |
This is not an acceptable fix. What didn't you answer my question first? ie what is the value of this->RenderWindow->IsCurrent() right before the call of glGetString(GL_VERSION) (line 484-485)? |
|
(0017093) Sean McBride (developer) 2009-08-11 11:00 |
I agree it's not a complete fix. But surely it's a necessary first step: sscanf crashes when given NULL, therefore one must test for NULL. I'm not sure what's objectionable there... (The reason I committed was because I'm cleaning/updating my working copy, and noticed that had been lying around for some time.) Sorry for not answering the previous question, I'll test it now... (rebuilding vtk...) |
|
(0017094) Sean McBride (developer) 2009-08-11 11:56 |
With no monitor connected, IsCurrent() returns 0; with a monitor it returns 1. |
|
(0017095) François Bertel (developer) 2009-08-11 12:56 |
OK. One more question: line 396, the code tests if this->RenderWindow is not null. Is this->RenderWindow null or not in the case with no monitor? |
|
(0017096) Sean McBride (developer) 2009-08-11 14:10 |
Please ask as many questions as you want, I would like to see this fixed afterall. :) this->RenderWindow is never null at line 396 either with or without monitor. |
|
(0017097) François Bertel (developer) 2009-08-11 15:19 |
OK. I committed a fix (rev 1.37) that should do better: 1. first check if there is a OpenGL context created and current for the calling thread 2. if not, return without doing any gl calls, as gl calls require an OpenGL context. Try it and let me know if it works with your headless Mac. Only after your feedback we will be able to change the status of the bug as fixed/closed. |
|
(0017122) Sean McBride (developer) 2009-08-13 15:46 |
The crash no longer happens in rev 1.37; thank you for the fix! I agree this is a more complete fix than what I did previously. However-- and I'm not trying to be a jerk here!-- I just don't understand why you removed the check for the null pointer. The man page for glString says: "If an error is generated, glGetString returns 0." Errors can occur for any number of reasons. Your fix prevents glString from returning 0 _in this case_ but there is no guarantee it won't return 0 for other reasons. |
|
(0017123) François Bertel (developer) 2009-08-13 16:11 |
I'm not preventing glGetString() to return 0 in this case. I'm preventing to calling a OpenGL function when there is no OpenGL context current to the calling thread. Regarding the OpenGL documentation, for some reason I don't understand, the OpenGL spec does not describe the return value and the kind of error that can happen... http://www.opengl.org/registry/doc/glspec21.20061201.pdf [^] (page 257) But the man page says the only errors that can be generated are when the name is not an accepted value (GL_VERSION is definitely an accepted value) or if glGetString() is called between glBegin() and glEnd(), which does not happen in our case. So we are good. http://www.opengl.org/sdk/docs/man/xhtml/glGetString.xml [^] |
| Notes |
| Issue History | |||
| Date Modified | Username | Field | Change |
| 2009-02-17 17:47 | Sean McBride | New Issue | |
| 2009-02-17 17:47 | Sean McBride | Status | backlog => tabled |
| 2009-02-17 17:47 | Sean McBride | Assigned To | => François Bertel |
| 2009-02-17 17:47 | Sean McBride | File Added: patch.txt | |
| 2009-02-18 13:17 | François Bertel | Note Added: 0015149 | |
| 2009-02-18 13:24 | Sean McBride | Note Added: 0015150 | |
| 2009-02-18 13:33 | Sean McBride | Note Edited: 0015150 | |
| 2009-02-24 17:50 | Sean McBride | Note Added: 0015354 | |
| 2009-02-27 16:15 | François Bertel | Note Added: 0015463 | |
| 2009-02-27 16:44 | Sean McBride | Note Added: 0015466 | |
| 2009-08-11 10:34 | Sean McBride | Note Added: 0017091 | |
| 2009-08-11 10:52 | François Bertel | Note Added: 0017092 | |
| 2009-08-11 11:00 | Sean McBride | Note Added: 0017093 | |
| 2009-08-11 11:56 | Sean McBride | Note Added: 0017094 | |
| 2009-08-11 12:56 | François Bertel | Note Added: 0017095 | |
| 2009-08-11 14:10 | Sean McBride | Note Added: 0017096 | |
| 2009-08-11 15:19 | François Bertel | Note Added: 0017097 | |
| 2009-08-13 15:46 | Sean McBride | Note Added: 0017122 | |
| 2009-08-13 16:11 | François Bertel | Note Added: 0017123 | |
| 2009-08-13 16:11 | François Bertel | Status | tabled => closed |
| 2009-08-13 16:11 | François Bertel | Resolution | open => fixed |
| 2011-06-16 13:11 | Zack Galbreath | Category | => (No Category) |
| Issue History |
| Copyright © 2000 - 2018 MantisBT Team |