osmo-pim-friends

[Osmo-pim-friends] [Patch] Make saving the config file safer

[Albrecht D. <alb...@ar...>]

Hi all,

I sporadically have the issue that the Osmo config file (config.xml) is truncated to 0 bytes, apparently when I log off.  Unfortunately, I cannot reproduce the issue (it "just happens" from time to time), so I can only presume that Osmo is terminated by the window manager (XFCE) while it is still writing the config, or that some other kind of subtle race exists.

The attached trivial patch tries to work around this issue by first writing the config to a temporary file (same name, by just appending '_'), which is then renamed to the "real" config file.  The latter operation shall be atomic according to IEEE 1003.1-2008.

I don't think it is necessary to pick up any left-over temp file, as it will not differ from the real file when it has been written while the X session was terminated.

Opinions?

Cheers,
Albrecht.

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[Albrecht D. <alb...@ar...>]

Am 06.04.17 08:38 schrieb(en) pasp:
> In revision 1048 the saving state of Osmo on SIGINT and SIGTERM has been implemented. Please check if the problem still exist for you.

I run svn rev. 1106 (2016-11-14 23:25:30) from the dbus branch, as I use Balsa which relies on Osmo as address book provider.  So this one should include this feature, shouldn't it?

> I use XFCE daily, and I didn't notice such problem.

As I mentioned, it happens sporadically only.  My home folder lives on a separate, LUKS encrypted partition, which /might/ make a difference.  Hmmm....

Thanks,
Albrecht.

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[Max G. <sf...@fu...>]

Hi Albrecht,

Could you please try the dbus branch HEAD from git.
I believe there was a small bug in the signal handling implementation in
1048.
@see 394887101d69735cb9c1fd77e9751d03aadcbc88

Thank you.
  Max

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[Albrecht D. <alb...@ar...>]

Hi Max:

Am 06.04.17 22:20 schrieb(en) Max Gordienko:
> Could you please try the dbus branch HEAD from git.
> I believe there was a small bug in the signal handling implementation in 1048.

Thanks, I updated to that rev., and removed my hack for testing.

Am 06.04.17 22:32 schrieb(en) Max Gordienko:
> That might be a wild theory but it looks like a race between Osmo signal handler and the LUKS umount process.

Thinking again about this theory, I guess its more likely that the signal is delivered to Osmo while it is running xmlSaveFormatFile().  I use XFS on top of LUKS, and an unmount race should thus be recovered from the journal.

I didn't look into the source of this function, but its is probably something like
(1) fopen(target_file, "w")
(2) write XML to the open file
(3) maybe fflush(), and fclose()

If Osmo catches the signal during step (2), any changes are not yet in the fs journal, so the result /must/ be the zero-length file.

> With your workaround (the underscore file) does it always work?

I did not see any issues with the patch, but without it I saw the issue without maybe every three or four weeks only.  I wouldn’t base a trustworthy statistics on that...

However, the "underscore" approach should be bullet-proof in the sense that rename() is (or should be) atomic - i.e. either the old or the new file is used.  This strategy is typically used for writing sensible files on embedded systems which are sometimes just powered off (without a proper shutdown process) while writing to flash, btw.  This guarantees that there is at least one intact copy after re-powering.

Cheers,
Albrecht.

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[pasp <pa...@qu...>]

Hi Albrecht.

> > Could you please try the dbus branch HEAD from git.
> > I believe there was a small bug in the signal handling implementation
> > in1048.  
> 
> Thanks, I updated to that rev., and removed my hack for testing.

Do you have any news on this issue? Just asking because I'm going to
release Osmo tomorrow.

Cheers,
Tomek

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[Albrecht D. <alb...@ar...>]

Hi Tomek:

Am 14.04.17 16:27 schrieb(en) pasp:
> > Thanks, I updated to that rev., and removed my hack for testing.
> 
> Do you have any news on this issue? Just asking because I'm going to release Osmo tomorrow.

Afaict, the latest "dbus" branch (commit 76ac5928794164824f06384cf6ce4ab18e86b098) runs without issues since a week or so.  So I guess Max was right that there was a problem with a specific dbus version.

Thanks a lot again for your great work,
cheers,
Albrecht.

Re: [Osmo-pim-friends] [Patch] Make saving the config file safer

[Max G. <sf...@fu...>]

Hi Albrecht,
> As I mentioned, it happens sporadically only.  My home folder lives on
> a separate, LUKS encrypted partition, which /might/ make a
> difference.  Hmmm....
>
That might be a wild theory but it looks like a race between Osmo signal
handler and the LUKS umount process.
With your workaround (the underscore file) does it always work?

  Max