View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0008554VTK(No Category)public2009-02-17 17:472009-08-13 16:11
ReporterSean McBride 
Assigned ToFrançois Bertel 
PrioritynormalSeveritycrashReproducibilityalways
StatusclosedResolutionfixed 
PlatformMac OS XOSMac OS XOS Version10.5
Product Version 
Target VersionFixed in Version 
Summary0008554: vtkOpenGLExtensionManager::ReadOpenGLExtensions derefs NULL on headless Macs
DescriptionvtkOpenGLExtensionManager::ReadOpenGLExtensions passes NULL to sscanf() on a Mac that has no connected monitor. This causes a crash.

Patch attached.

TagsNo tags attached.
Project
Type
Attached Filestxt file icon patch.txt [^] (775 bytes) 2009-02-17 17:47 [Show Content]

 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 [^]

 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)


Copyright © 2000 - 2018 MantisBT Team