fitzsim suggested that not all functions be synchronized, but there be a
synchronized block around the code that adds the new pointer to the
groupMap.
Fixed.
2006-06-30 Lillian Angel <[EMAIL PROTECTED]>
* gnu/java/awt/peer/gtk/GtkCheckboxPeer.java
(create): Changed to be non-synchronized.
(setLabel): Likewise.
(setCheckboxGroup): Likewise.
(addToGroupMap): Likewise. Added synchronized block around
code.
(dispose): Changed to be non-synchronized.
On Fri, 2006-06-30 at 15:31 -0400, Lillian Angel wrote:
> Hi,
>
> I fixed the issues Tom stated below.
>
> On Fri, 2006-06-30 at 13:03 -0400, Thomas Fitzsimmons wrote:
> > Hi,
> >
> > Lillian Angel wrote:
> > > This patch fixes the long standing problem of not being able to
> > > dynamically switch between a checkbox and a radio button. I.e. moving a
> > > checkbox to a checkbox group changes the checkbox to a radio button.
> > >
> > > Tom helped a lot with this patch. He removed the CheckboxGroupPeer and
> > > we fixed it so everything is handled in GtkCheckboxPeer.
> > >
> > > There is a mauve test for this. The harmony test
> > > (test.java.awt.CheckboxTest) now passes.
>
> [...]
> > > + if (groupPointer != 0)
> > > + {
> > > + native_group = JLONG_TO_PTR (GSList, groupPointer);
> > > + if(! GTK_IS_RADIO_BUTTON (native_group->data))
> > > + native_group = NULL;
> >
> > When does it happen that the data field is not a GtkRadioButton? I suspect
> > only
> > after removing a Checkbox from a CheckboxGroup? This is likely a memory
> > leak --
> > if the native_group's data field is no longer valid then we should probably
> > free
> > the GSList.
>
> Gtk actually frees the list when needed. I changed these checks to
> asserts and set the native_group to null in *_removeFromGroup (when
> needed).
>
> [...]
>
> > > + gdk_threads_leave ();
> > > +
> > > + return PTR_TO_JLONG (native_group);
> >
> > I realized that there is a race condition here. Instead of returning the
> > pointer value, we should use JNI to set the groupPointer field with the GDK
> > lock
> > held.
>
> I changed all functions in GtkCheckboxPeer to be syncronized, and added
> a new function to put the new group in the groupMap. This function is
> called from the native peer.
>
> [...]
> > > + NSA_DEL_PTR (env, obj);
> > > + NSA_SET_PTR (env, obj, container);
> >
> > The NSA table points to the containing event box, so I don't think these
> > two
> > lines are necessary.
>
> Removed
> >
> 2006-06-30 Lillian Angel <[EMAIL PROTECTED]>
>
> * gnu/java/awt/peer/gtk/GtkCheckboxPeer.java:
> Changed all return values of native functions to void.
> (create): Changed function to be syncronized. Removed
> call to put value in groupMap, this is now done from
> the native code.
> (setState): Changed function to be syncronized.
> (setLabel): Changed function to be syncronized.
> (setCheckboxGroup): Changed function to be syncronized. Removed
> call to put value in groupMap, this is now done from
> the native code.
> (postItemEvent): Changed function to be syncronized.
> (addToGroupMap): New function. Called by native code to add
> new value to the group.
> (dispose): Changed function to be syncronized.
> * include/gnu_java_awt_peer_gtk_GtkCheckboxPeer.h: Updated
> all functions.
> * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkCheckboxPeer.c
> (cp_gtk_checkbox_init_jni): Added code to link to
> java function.
> (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_createRadioButton):
> Changed return value to void. Added call
> to java function to set pointer in groupMap.
> (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_addtoGroup):
> Likewise. Also, changed check to an assert. Also, removed call
> to set/del pointer.
> (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_removeFromGroup):
> Likewise. Also, added check to determine if native_group should
> be set to NULL.
> (Java_gnu_java_awt_peer_gtk_GtkCheckboxPeer_switchToGroup):
> Likewise.
>
> Committed,
> Lillian
Index: gnu/java/awt/peer/gtk/GtkCheckboxPeer.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/GtkCheckboxPeer.java,v
retrieving revision 1.27
diff -u -r1.27 GtkCheckboxPeer.java
--- gnu/java/awt/peer/gtk/GtkCheckboxPeer.java 30 Jun 2006 19:28:21 -0000 1.27
+++ gnu/java/awt/peer/gtk/GtkCheckboxPeer.java 30 Jun 2006 19:52:25 -0000
@@ -80,7 +80,7 @@
super (c);
}
- public synchronized void create ()
+ public void create ()
{
Checkbox checkbox = (Checkbox) awtComponent;
current_group = checkbox.getCheckboxGroup ();
@@ -133,12 +133,12 @@
}
}
- public synchronized void setLabel (String label)
+ public void setLabel (String label)
{
gtkButtonSetLabel (label);
}
- public synchronized void setCheckboxGroup (CheckboxGroup group)
+ public void setCheckboxGroup (CheckboxGroup group)
{
if (current_group == null && group != null)
{
@@ -221,12 +221,15 @@
}
}
- public synchronized void addToGroupMap(long groupPointer)
+ public void addToGroupMap(long groupPointer)
{
- groupMap.put(current_group, new Long (groupPointer));
+ synchronized (groupMap)
+ {
+ groupMap.put(current_group, new Long (groupPointer));
+ }
}
- public synchronized void dispose ()
+ public void dispose ()
{
super.dispose ();
}
