[Marble-devel] Review Request: Clean gpsd position provider shutdown

[Dennis Nienhüser]

> On 2010-04-28 06:57:26, Bastian Holst wrote:
> > /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp, line 115
> > <http://reviewboard.kde.org/r/3794/diff/1/?file=24418#file24418line115>
> >
> >     I'm not sure if it is a good idea to remove this. Thinking that this will never happen it is no problem to do this, but otherwise it would result into deleting a running QThread which seems to be worse according to the Qt doc as it would result in a crash.

Good point, I changed it to only delete the thread if it exited correctly.


- Dennis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3794/#review5276
-----------------------------------------------------------


On 2010-04-23 19:38:29, Dennis Nienhüser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3794/
> -----------------------------------------------------------
> 
> (Updated 2010-04-23 19:38:29)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> When gpsd is running, but no data is available, the gpsd position provider is currently blocked indefinitely for libgsp >= 2.90 by the poll() call. In that case the destruction of the thread hangs in wait() after terminate(), which leads to marble freezing when turning gpsd off. It may also cause the crash reported in https://bugs.kde.org/show_bug.cgi?id=234311.
> 
> The patch changes the behavior to only call the blocking method when data is available (and it won't block in that case). Additionally it removes the possibly dangerous (according to qt docs) terminate() call.
> 
> 
> This addresses bug 234311.
>     https://bugs.kde.org/show_bug.cgi?id=234311
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdConnection.cpp 1117994 
>   /trunk/KDE/kdeedu/marble/src/plugins/positionprovider/gpsd/GpsdPositionProviderPlugin.cpp 1117994 
> 
> Diff: http://reviewboard.kde.org/r/3794/diff
> 
> 
> Testing
> -------
> 
> Using libgps-2.92-4 and some gps device supported by gpsd.
> 
> 
> Thanks,
> 
> Dennis
> 
>