Bug 60690 - kill #ifdef HORRIBLE_OBSOLETE_YIELDMUTEX_IMPL
Summary: kill #ifdef HORRIBLE_OBSOLETE_YIELDMUTEX_IMPL
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
4.0.0.3 release
Hardware: Other All
: medium normal
Assignee: Yousef Hamza
URL:
Whiteboard: target:4.1.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2013-02-11 20:34 UTC by Michael Meeks
Modified: 2015-12-15 23:56 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2013-02-11 20:34:16 UTC
Remove all the code in these sections, and simplify:

git grep HORRIBLE_OBSOLETE_YIELDMUTEX_IMPL

Presumably there is some degree of simplification / removal of inheritance that can be achieved by doing that. It'd be nice to cleanup that code.

Thanks ! :-)
Comment 1 Michael Meeks 2013-02-11 20:36:24 UTC
Oh ! I forgot, we should also add a check for a recent(ish) version of gtk+ to our gtk2 plugin to ensure that we are not loaded on a truly ancient gtk+ that does not have the infrastructure needed for:

gdk_threads_set_lock_functions

We should also cleanup the horrible dlopening hackery that happens in:

    static bool hookLocks( oslModule pModule )
    {
        typedef void (*GdkLockFn) (GCallback enter_fn, GCallback leave_fn);

        GdkLockFn gdk_threads_set_lock_functions =
                (GdkLockFn) osl_getAsciiFunctionSymbol( pModule, "gdk_threads_set_lock_functions" );
        if ( !gdk_threads_set_lock_functions )

We should just directly link to gdk_threads_set_lock_functions instead :-)

And while we include that we should use

#if !GTK_CHECK_VERSION(2,2,0) // or whatever version it is
#  error "no lock hooking !"
#endif

to check that at compile time too.

Thanks !
Comment 2 Omar christopher Eugene 2013-02-12 12:23:18 UTC
Quick question: How do I check if my code solves the issue? I am very new to this?
Comment 3 Michael Meeks 2013-02-12 13:00:25 UTC
Just mail in a patch :-) if it compiles and gtk2 still works (it'd be good to check that) then life is good. Otherwise, it's up to the reviewer.

Thanks :-)
Comment 4 Vlastimil Jinoch 2013-02-23 08:38:41 UTC
Hi,
I'm student of Faculty of Electrical Engineering on CTU. The goal of my semestral work in the subject Open source programming is to join some existing project and do some useful tasks to improve or extend that project.  So I selected this project and this task and I'll do it as soon as possible.
Comment 5 Vlastimil Jinoch 2013-04-09 14:19:09 UTC
Hi,

I push a patch to gerrit https://gerrit.libreoffice.org/3289
Comment 6 Vlastimil Jinoch 2013-04-11 21:23:01 UTC
Hi,

new commit https://gerrit.libreoffice.org/3349
Comment 7 Michael Meeks 2013-04-12 10:10:27 UTC
Great - I pushed that. There is more cleanup potential here though ... so lets not close this bug yet.

Incidentally - can you put fdo#60690 as the first word in your commit messages so they are associated with here ? here is what else needs doing ;-)

These methods need killing:

    virtual int         Grab()          { return 0; };
    virtual void        Ungrab(int )    {};

And all their uses; which rather highlights the fact that "GTK_YIELD_GRAB" has actually not done anything for ages - and we should remove all instances of that macro, and all it's call sides.

It was an horrible fallback for the proper integrated locking that we could do with the hook :-)

Thanks !
Comment 8 Commit Notification 2013-04-12 10:11:02 UTC
Michael Meeks committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a0757f0e0fb4fe4b837f44b2bc9ca6455fa1c5d

fdo#60690 - a little more cleanup of gtk+ mutex bits.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 Vlastimil Jinoch 2013-04-12 13:28:16 UTC
Hi,

I push it on: https://gerrit.libreoffice.org/3357
Comment 10 Commit Notification 2013-04-12 17:10:24 UTC
vjinoch committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a1cd39a17216d78b4f335e6301786e205be14d0d

fdo#60690 - Remove all calls t GTK_YIELD_GRAB because it does nothing.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Michael Meeks 2013-04-12 17:29:00 UTC
Thanks for the great work, we can mark this fixed now :-)
Comment 12 Robinson Tryon (qubit) 2015-12-15 23:56:11 UTC
Migrating Whiteboard tags to Keywords: (EasyHack,DifficultyBeginner,SkillCpp,TopicCleanup)
[NinjaEdit]