Bug 606

Created attachment 363 [details]
AWT Processing error log

something is very very odd w/ your logfile!

the stack-trace doesn't match with the source code revision 
as stated in it:
   jogl.build.number=699
   jogl.build.id=2012-03-08_15-56-10
   jogl.build.branch=master
   jogl.build.commit=d28e1b139f14b10b9e22750ac44dbc18f08a0d34

I even dl the build and checked w/ git .. it's the correct sha1 / source,
but no line numbers (e.g. within GLCanvas match).

Exception in thread "Animation Thread" main: setRealized: MacOSXPbufferCGLDrawable false -> true
java.lang.NullPointerException
	at javax.media.opengl.awt.GLCanvas.validateGLDrawable(GLCanvas.java:555)
	at javax.media.opengl.awt.GLCanvas.display(GLCanvas.java:398)

But source says:
 555         // before native peer is valid: X11
 556         disableBackgroundErase();
 557 
 558         // issues getGraphicsConfiguration() and creates the native peer
 559         super.addNotify();
.. 
and:
 395   public boolean isRealized() {
 396       drawableSync.lock();
 397       try {
 398         return ( null != drawable ) ? drawable.isRealized() : false;
 399       } finally {
 400         drawableSync.unlock();
 401       }
 402   }

also the mentioned sha1 / build contains the locking code.

is it possible that you have 'edited' the file ?

Or is it too later for me tonight ?

+++

Also .. I could not run the svn checkedout processing :(
I cloned it, ran 'ant run' to build/run .. OK, but then no GLEventListener found.
I guess the classpath isn't setup ?
Me even tried to build a dist .. but no jogl jar's referenced this way.
Pls advise .. since I need to double check the locking code.

(In reply to comment #2)
> something is very very odd w/ your logfile!
> 
> the stack-trace doesn't match with the source code revision 
> as stated in it:
>    jogl.build.number=699
>    jogl.build.id=2012-03-08_15-56-10
>    jogl.build.branch=master
>    jogl.build.commit=d28e1b139f14b10b9e22750ac44dbc18f08a0d34
<http://jogamp.org/git/?p=jogl.git;a=commit;h=d28e1b139f14b10b9e22750ac44dbc18f08a0d34>

> 
> I even dl the build and checked w/ git .. it's the correct sha1 / source,
> but no line numbers (e.g. within GLCanvas match).
> 
> Exception in thread "Animation Thread" main: setRealized:
> MacOSXPbufferCGLDrawable false -> true
> java.lang.NullPointerException
>     at javax.media.opengl.awt.GLCanvas.validateGLDrawable(GLCanvas.java:555)
>     at javax.media.opengl.awt.GLCanvas.display(GLCanvas.java:398)
> 

<http://jogamp.org/git/?p=jogl.git;a=blob;f=src/jogl/classes/javax/media/opengl/awt/GLCanvas.java;h=d8d9ddf6b0dcc921a819b2ca028341013d7dfe48;hb=efa70cd39e1a2ac18c3e8660f8d57e4569b19018#l555>

<http://jogamp.org/git/?p=jogl.git;a=blob;f=src/jogl/classes/javax/media/opengl/awt/GLCanvas.java;h=d8d9ddf6b0dcc921a819b2ca028341013d7dfe48;hb=efa70cd39e1a2ac18c3e8660f8d57e4569b19018#l398>

Sorry, maybe I did something wrong when "downgrading" to the earlier build?

Since I originally checked out the latest revision for both jogl and gluegen, I tried older revisions by running the following command:

git reset --hard <commit id>

For instance, in order to test the build that I think created the problem, I did:

git reset --hard 3ed491213f8f7f05d7b9866b50d764370d8ff5f6

Is this the correct way to do it?

With regards to the Processing testing, it turns out that Ben Fry started just yesterday to move the opengl library directly into the core packages, so the most recent revisions from the trunk might not be fully functional yet. I just asked him if the porting is completed or if he is still working on it. I will let you know.

(In reply to comment #4)
> Sorry, maybe I did something wrong when "downgrading" to the earlier build?
> 
> Since I originally checked out the latest revision for both jogl and gluegen, I
> tried older revisions by running the following command:
> 
> git reset --hard <commit id>
> 
> For instance, in order to test the build that I think created the problem, I
> did:
> 
> git reset --hard 3ed491213f8f7f05d7b9866b50d764370d8ff5f6
> 
> Is this the correct way to do it?
> 

This should work, but you probably need to delete all builds ('rm -rf build*').

What I do to bisect things [manually] is to create branches at commit points,
check them out to use it, delete the build folder, build and test.

> With regards to the Processing testing, it turns out that Ben Fry started just
> yesterday to move the opengl library directly into the core packages, so the
> most recent revisions from the trunk might not be fully functional yet. I just
> asked him if the porting is completed or if he is still working on it. I will
> let you know.

Thx.

So .. 2 hours went to a full review of all GLAutoDrawable locking code (AWT, SWT, NEWT, ..)
considering code changes and remarks:
  3ed491213f8f7f05d7b9866b50d764370d8ff5f6
  1a91ec5c8b6fd9d9db7bc115569c369fe7b38e9b
  3334a924309a9361a448d69bc707d4cce416b430
  4f27bcecf7484dc041551f52a5c49e2884cb3867

It seems necessary to have proper locking employed for all
semantic actions changing, creating and destroying drawable/context.
To avoid deadlock, we have to ensure the locked code segment will not spawn 
off to another thread, ofc.
Other read-only methods at least shall utilize a safe local copy of a volatile
if further use to produce the result is necessary.

I have implemented the above pattern (all GLAutoDrawable*) and currently 
testing it. Will ping you when pushed, so you can test - which would be very helpful.

  - create

http://jogamp.org/git/?p=jogl.git;a=commit;h=4b5a0f6557d7152ec770bc13ad3c494449de0529

    Considering code changes and remarks:
      3ed491213f8f7f05d7b9866b50d764370d8ff5f6
      1a91ec5c8b6fd9d9db7bc115569c369fe7b38e9b
      3334a924309a9361a448d69bc707d4cce416b430
      4f27bcecf7484dc041551f52a5c49e2884cb3867
    
    It seems necessary to have
    
    - recursive locking employed for all semantic actions which changes drawable & context 
      (and the Window resource)
    
    - to avoid deadlock, we have to ensure the locked code segment will not spawn
      off to another thread, or a thread holds the lock, spawns of an action requiring the lock. .. sure
    
    - other read-only methods (flags, ..) shall at least utilize a safe local copy of a volatile field
      if further use to produce the result is necessary.
    
    - flags like sendReshape require to be volatile to guarantee it's being processed
    
    Patch impacts: AWT/SWT GLCanvas, GLAutoDrawableBase [and it's specializations]
    and hopefully closes any loopholes of missing a cache hit, etc.
    
    If you review this and find optimizations, i.e. removing a lock due to semantics etc,
    don't hold back and discuss it, please.

Appears to work very nicely. The crash is now gone. Thanks!

I found an issue that seems to be triggered by the changes of this commit: the application's framerate won't go over 60fps, even it trying explicitly from the code to make it run faster...

(In reply to comment #8)
> I found an issue that seems to be triggered by the changes of this commit: the
> application's framerate won't go over 60fps, even it trying explicitly from the
> code to make it run faster...

Can you elaborate on this ?
  - platform
  - test case 
  - .. ?