Cookies file randomly reset

[shioyama]

I can't pinpoint the trigger, but in my setup the cookies file seems to get randomly reset, such that I lose all my cookies and have to re-login to all sites if I open a new window (existing windows retain their session cookies and continue to work).

This was discussed in #154 but I have fixed the issues related to the values of cookie-expire-time and cookie-timeout, and this does not seem to be related AFAICT. I've reproduced the issue on two separate laptops (both running arch linux, both running vimb 2.10 (although slightly different commits)).

For now I'd just be happy to hear if anybody else has encountered the same issue, otherwise it is possibly due to my particular setup (both laptops have the same dotfiles, packages, etc. so almost identical). Any other clues to help debug this would be appreciated, I'm at a loss for how to figure out what's going on.

[fanglingsu]

I think you should provide more information what to do to run into your issue. The information you provide here are not enough to know when and after which time the cookie file is reset.

• When does the issue appear?
• Which steps should one do to provoke the cookie reset?
• Are the cookies reset right after opening a new instance of vimb or some time later? After which time?
• Are all the cookies in the file reset, or only those that belong to the current used domain?
• ...

[shioyama]

Sorry I realize this is not enough information to debug this issue. I am having a lot of trouble isolating the moment when it happens since I usually don't notice until much later (since session cookies on existing instances of vimb remain logged in, and new windows on other pages do not show any obvious signs until I try to do something related to a logged-in service).

At the moment I'm hoping that someone can just confirm that it's not just me, so I can figure out if it is specific to my environment.

Which steps should one do to provoke the cookie reset?

I'm trying to figure this out, so far I can't isolate the exact conditions.

Are the cookies reset right after opening a new instance of vimb or some time later? After which time?

They appear to be reset when I open a new window, although as noted above I can't be sure since I only notice when I open a new window.

Are all the cookies in the file reset, or only those that belong to the current used domain?

All cookies. It seems the file itself is just written over.

[infmagic2047]

I'm having the same problem here. It seems to happen randomly when page is loading.

It happened once just now. When I refresh this page (I didn't test other pages), all my cookies disappears. I tried to restore the cookies from backup and refresh again, and the cookies are reset almost every time. However, if I closed all browser windows and do that again, the cookies are still there.

When the cookies are reset, the cookies file grows a lot in 0.5s, and then become empty. This may be a hint to find the bug.

[fanglingsu]

@yyt16384 Sorry for the late response. Is it right that the cookies file is reset on you system with only instance/window of vimb running? This would make it easier to reproduce and to test.

May be the issue is something related to the filesystem. I've ext4 for the config directory where the cookies files resists and never seen the cookies file to be reset.

[fanglingsu]

@yyt16384 Maybe you can use strace to check when the cookies files is reset. I've not teste, but somethings like this should work strace -e trace=open,write,access -f -o trace.log vimb.

[shioyama]

@yyt16384 really happy to hear that I'm not the only one dealing with this bug! Hopefully together we can pinpoint the cause.

@fanglingsu I'll give that a shot and see. I can't seem to find any common factor, it happens on both of my workstations (very similar) although less on one than the other, but for no obvious reason.

[shioyama]

Hmm trying to test this with trace on and finding now it's not happening anymore. I just did a full system upgrade so maybe it was something system-related.

@yyt16384 are you still having this issue? What's your system like? I'm using i3 window manager on arch linux, latest kernel etc.

[shioyama]

@fanglingsu Looking at the trace log, I do notice a lot of ENOENT (No such file or directory), not sure if that is normal. Although as I commented above, I'm not seeing the issue now so maybe not related.

[shioyama]

Never mind spoke too soon, it happened again. I have the trace but hard to pick out anything, @fanglingsu what should I be looking for?

[fanglingsu]

@shioyama My approach would be to look for write operations to the cookie file are attempt to get a lok on it. But I'm not as experienced in reading such traces, so I'd scroll to the trace an look for some strance parts in it.

[infmagic2047]

I'm not familiar with file locks, but if I'm reading man fcntl correctly, there are some problems in the locking code.

1. fcntl accepts a struct flock * for locking, but a struct flock is passed.
2. We should use F_SETLKW instead of F_SETLK.
3. To place a read/write lock, the file descriptor must be open for reading/writing, respectively.

I haven't tested it, but I did a experiment and it seemed to work after fixing these things.

[fanglingsu]

@yyt16384 Thank you for you investigation according to the man page you are right. The fcntl should get a pointer to the struct, but why does the compiler not mention this issue? And the F_SETLKW sounds like a better approach independent if this fixes the issue or not.
Can you please provide a pull request with you changes?

[infmagic2047]

I have made a pull request about this.

fcntl takes variable number of arguments, and the compiler is not intelligent enough to warn about this.

[shioyama]

@fanglingsu @yyt16384 Really happy to see this fix, but unfortunately although the problem seems to happen less often, I'm still seeing it happen with the latest version of vimb. I'll try tracing it down.

[infmagic2047]

@shioyama I'm also seeing it happen here.

I don't have time to work on it now, but I think it would be to useful to add some logging to cookiejar_changed in cookiejar.c.

If you can build a custom libsoup, you can also try to add logging to soup_cookie_jar_text_changed, delete_cookie and possibly write_cookie in soup-cookie-jar-text.c, as it's where the actual IO happens.

[shioyama]

@yyt16384 thanks glad we seem to be seeing the same behaviour. I'll see what I can do, my c is a bit rusty but eager to get this fixed so I'll give it a shot.

@fanglingsu sorry can we re-open this?

[fanglingsu]

@shioyama If the issue still exists it's always a good choice to reopen the issue. Sorry that I can't really help here. I can't reproduce the issue neither on my arch linux nor on the ubuntu system of my wife. So I was glad that Yutao Yuan spent some time of the nice and important patch, that made me think it's now fixed with it.

[shioyama]

@fanglingsu no worries! I didn't mean to sound like I was complaining. I really appreciate what you've done creating vimb and for how responsive you are to issues.

This is a nasty problem because it's so hard (for me at least) to reproduce reliably... I wonder if some particular pages/actions/environments trigger it more often. I usually have 10-20 vimb windows open at once logged into different services, but still can't isolate any that are more likely to trigger it.

[shioyama]

Looking at the manpage for fcntl, it looks like if the return value is -1 then the lock was not granted and so nothing should be written to the file, but the return value is never used currently in vimb (here, here, here, here, here and here it is called but the result is ignored).

The change from F_SETLK to F_SETLKW in c5b2065 would make the thread wait until the lock is freed, but the manpage also says:

If a signal that is to be caught is received while fcntl() is waiting for a region, fcntl() shall be interrupted. Upon return from the signal handler, fcntl() shall return −1 with errno set to [EINTR], and the lock operation shall not be done.

Since this return value is being ignored, it would seem to me that vimb in this case would just go ahead and write to the file anyway and this could cause the issues we've seen. Just a guess, I'll try testing it out and see if it's the cause of the problems here.

[shioyama]

No dice. I put a debugger message to check if the return value was -1 anywhere, but it never fired even though the cookies file was reset...

[shioyama]

@fanglingsu I'm reading through the SoupCookieJarDB since it seems to be the closest reference to what vimb does with file storage of cookies. Still not sure exactly where things are going wrong but one thing I'm curious about: is there any reason you didn't decide to just use mozilla-type sqlite3 storage, as provided by that class? Then you wouldn't have to re-invent the wheel with file locks etc. I'm leaning now toward trying to just try that and see if it works out of the box.

[shioyama]

So I'm testing now with sqlite for cookies and it seems to be working fine, and only required changing one line. I'm not 100% sure yet that it fixes the problem but at the very least if the problem re-occurs, I will know that it has nothing to do with cookiejar.c/cookiejar.h.

[fanglingsu]

@shioyama Thank you for your investigation.

Still not sure exactly where things are going wrong but one thing I'm curious about: is there any reason you didn't decide to just use mozilla-type sqlite3 storage, as provided by that class?

First I like to use files because ist a little easier to process it wit external tools. But now it's required to get the external download working. This would spawn a downloader that might know your current cookie information to use you authenticated session. As I know, does the curl and wget not support sqlite as cookie storage, so it's required for this to have the cookie file.
I have to admit that I spent some of the cookie related code from surf and dwb and spent not much time to check if the locking is done right. When I look to the code now, I'm not really sure if the cookiejar.c is required. Maybe it's enough to let soup do the dirty things with the file instead.

[infmagic2047]

I think cookiejar.c modifies cookies according to cookie-timeout and cookie-expire-time before sending them to libsoup.

[shioyama]

@yyt16384 yes existing timeout functionality does not work exactly as-is with the sqlite cookies file. However I don't want my cookies to expire anyway so it doesn't really matter.

I've tested with sqlite now for a few hours and I don't see the issue here happening at all, so I'm going to stick with this setup since it works for my needs. @fanglingsu I also don't see any issue with downloading even where cookies would be required, not sure why but just happy everything seems to work.

So overall I think if we really can't nail down the source of this bug I'd suggest having an option to use sqlite3 cookies. Also this could possibly have the added benefit of sharing cookies with firefox which uses the same format (although I have not tested that one).

[fanglingsu]

I also don't see any issue with downloading even where cookies would be
required, not sure why but just happy everything seems to work.

The cookie file is only needed for the external download set downloadcommand=... when download-use-external is enabled. Else vimb or better webkit does the download and there is no need to say where the cookies are.

So overall I think if we really can't nail down the source of this bug I'd
suggest having an option to use sqlite3 cookies. Also this could possibly
have the added benefit of sharing cookies with firefox which uses the same
format (although I have not tested that one).

Because of the issue with the external download, it would not really make sense to use sqlite. OK, we could allow to use it and inform that this would not work with curl or wget (other tools might also be able to use the sqlite). The sharing of cookies with firefox is no argument for me and I don't advice to share cookies betwenn different browsers. Most of the serious webapplications check the session against some browser specific data like user-agent String. If you open a session with one browser and try to pick it up with another one, the webapplication would interprete this as an session hijacking attempt and clear the session.

But your investigation helped to strip down the issue to the file storage of the cookies or to the locking. Could you also try to remove the FLOCK() in cookiejar.c. So we can check if the issue is related to the locking or exists in general in the cookie file storage.

[shioyama]

Because of the issue with the external download, it would not really make sense to use sqlite. OK, we could allow to use it and inform that this would not work with curl or wget (other tools might also be able to use the sqlite).

Fair enough, I just was hoping this might help someone.

The sharing of cookies with firefox is no argument for me and I don't advice to share cookies betwenn different browsers. Most of the serious webapplications check the session against some browser specific data like user-agent String. If you open a session with one browser and try to pick it up with another one, the webapplication would interprete this as an session hijacking attempt and clear the session.

Right sorry, I was just throwing that idea out there but you're right it doesn't make much sense.

But your investigation helped to strip down the issue to the file storage of the cookies or to the locking. Could you also try to remove the FLOCK() in cookiejar.c. So we can check if the issue is related to the locking or exists in general in the cookie file storage.

Sure I can try that. The problem with this bug is that it's hard to reproduce reliably so I need to actually spend at least an hour or two in regular use to get it to happen, which means in the end losing time with cookies getting reset. But I do want to fix this problem and I think we're getting close so I'll give it a shot and report back.

[fanglingsu]

However I don't want my cookies to expire anyway so it doesn't really matter.

Note that the expire-time stuff was added to pick up sessions in case you open a page into a new window, that is a new instance of vimb and does not share any memory or cookie information except of the cookies that are written to the cookies file. So if the webserver attemps to set a cokie without lifetime this would normally saved in memory and could not be picked up by another browser instance or the external download. So, in case such an session cookie is set, we force libsoup to write it into the file.

[shioyama]

Ok I've done a bit of investigating. I tried removing the cookiejar_changed method altogether (just let it call SOUP_COOKIE_JAR_CLASS(..)->changed and that didn't work. Then tried removing all the FLOCK calls and that didn't work either (still seeing same bug).

I'm a bit at a loss at what could be left, unless the issue is that locking is necessary but is not being performed correctly, which would be tricky to debug...

[fanglingsu]

Hm, we could try an attempt without the cookiejar.c and use soup_cookie_jar_text_new(vb.files[FILES_COOKIE], FALSE); in main.c at in session_init(). If this fails too, this issue is related to cookie file implementation of libsoup.

[shioyama]

@fanglingsu ok I'll give that a shot.

[shioyama]

Ok this seems to be an issue with soup cookiejar text cookies in general, I'm still seeing the issue with @fanglingsu's suggestion above. @yyt16384 could you try the same thing just to sanity check this? Nobody else seems to be encountering it except for us.

[infmagic2047]

Well, locking is required here as libsoup does no locking itself with text cookies storage.

I just looked into the source code of surf, and the only real difference is that we use fcntl for locking instead of flock. It seems that simply replacing it causes some problems.

fcntl locks are released automatically when the process closes any file descriptor referring to the file. Since libsoup opens and closes the file every time it writes cookies, the locking is not going to work.

One possible solution would be to change it to flock. But it was changed to fcntl in 4745515 to fix compilation on FreeBSD, so it would not be a good idea to change it back.

[shioyama]

Thanks @yyt16384 that sounds like we have an explanation for this now. Maybe fixing compilation on freebsd would be easier than debugging locking issues...

[shioyama]

For now I've added back the cookiejar_changed stuff that saves session cookies to my fork so I can use sqlite cookies and have them persisted across vimb processes, and seems to be working well. I'll try also reverting 4745515 and see if that fixes the problem with text file-based cookies.

[fanglingsu]

@yyt16384 You are right the only reason I switched to fcntl was a build error on FreeBSD. I don't have a FreeBSD system to test this, but according to FreeBSD flock man page flock should be available there too.

So I think we should give the flock an other try and try to fix it for FreeBSD if this will still be an issue.

[fanglingsu]

I'll try to provide a branch to test this using flock instead of fcntl soon.

[fanglingsu]

So, I've reverted to use flock and put this into new branch fix-file-locking.
@shioyama can you please test this branch?

[fanglingsu]

Good news, I could reproduce the issue with github account.

• I cleaned the cookies file
• logged in to github
• and than opened github into multiple new instances of vimb.

During the load of the windows the cookies file was temporary empty. With the new branch the cookies file was kept like expected. But maybe we have to spent more time to check if it happens again.

[shioyama]

@fanglingsu great to hear! I'll test with the flock branch asap and get back to you.

Funny thing I also noticed that when the file was cleared, the entry at the top mostly ended up being github. Maybe something about github is more likely to trigger it...

[shioyama]

I've been running with the fork on my home setup for the last week without a single case of cleared cookies, so I think we can safely assume that using flock solves the issue. Only thing remaining then is to figure out what the issue was with compiling on FreeBSD. Can't really be of much help there... but honestly I think we should just revert 4745515 and wait for someone who uses FreeBSD to fix it.

[fanglingsu]

OK, I'll merge the branch into master.

[fanglingsu]

The fix-file-locking branch is now merged into master.

[shioyama]

Thanks!

[tgalal]

still happens to me on vimb 3.1.0, 2 different computers running archlinux

[fanglingsu]

@tgalal I've opened the new issue #470 because we switche the webkit version in between so this might be something different. Could you please post some more information helping to reproduce the issue there?