Hi, On Tue, 2005-08-16 at 20:00 +0200, Mark Wielaard wrote:
> I would like for someone to review how I handled the native/gtk > lock/thread transitions. Since the datatransfer api is basically > synchronous, but the gtkclipboard is asynchronous we need to call into > gtk+ to setup callbacks and then wait till those trigger before we can > return the results to the user. This is slightly complicated by the fact > that only one outstanding selection request is allowed. Where possible > the code tries to sensibly cache the results of any datatransformation > it has done. I looked over the locking and threading in this patch. It is safe, except for the case that you identified. > There is one known issue and that is with the > GtkClipboard.provideContent() methods. They are called from a callback > (and are holding the main gtk lock) but call arbitrary user code through > DataTransfer.getTransferData(). This could potentially deadlock if that > user code somehow calls back into gtk+. To solve this properly we need a > change in the gtk+ clipboard/selection code to allow us to provide the > data asynchronous. Yes, please discuss this with Owen Taylor and file bugs against GTK if necessary. This is a relatively new API and we should provide feedback where it doesn't meet our needs. > > I have committed it already since it is a huge improvement over the old > (buggy, non-working) code. If you want to play with it try the new > datatransfer demo example gnu.classpath.examples.datatransfer.Demo. > Comments on coding style, rants on lock/threading issues or other issues > you can find in the code are appreciated. Just two requests for the native code. First, please create separate C files for GtkClipboard and GtkSelection. We've tried lumping native implementations together in the past but it's only caused confusion. Second, we shouldn't prefix static symbols with cp_gtk_; instead cp_gtk_ should be reserved for exported symbols. Otherwise this is excellent. Thanks, Tom _______________________________________________ Classpath-patches mailing list Classpath-patches@gnu.org http://lists.gnu.org/mailman/listinfo/classpath-patches