Exiv2

hi Robin,

thank you for your time and your efforts … not sure if my email reply made it into redmine, so i'll post it here again:

Your server is Windows. On what platform are you using pyexiv2?

we are using pyexiv2 on a debian wheezy box. both pyexiv2 and libexiv2
are straight from the debian repositories, libexiv2 is 0.23-1. the same
applies to cifs-utils …
everything is fine when pyexiv2 is writing to a share provided by a
samba4 server - so yeah, the root cause could well be some subtle
differences in the cifs implementation between samba and windows.

I use pyexiv2 a lot. Almost all 50,000 photographs on my web site
are processed by a python script. I'm sympathetic that the
underlying issue is in libexiv2, however you'll understand that I'm
not going to get involved with supporting pyexiv2.

fair enough :-)
i can reproduce the issue here quite reliably, and it occurs with exiv2,
too:

KMT\adlibworker@stone:~$ mount | grep win2k
//shatner.server.kinemathek.de/images on
/home/adlibworker/win2k-share-with-cache type cifs
(rw,relatime,vers=1.0,cache=strict,username=adlibworker,domain=KMT,uid=10047,forceuid,gid=10000,forcegid,addr=172.20.1.61,file_mode=0755,dir_mode=0755,nounix,serverino,rsize=61440,wsize=65536,actimeo=1)
//shatner.server.kinemathek.de/images on
/home/adlibworker/win2k-share-without-cache type cifs
(rw,relatime,vers=1.0,cache=none,username=adlibworker,domain=KMT,uid=10047,forceuid,gid=10000,forcegid,addr=172.20.1.61,file_mode=0755,dir_mode=0755,nounix,serverino,rsize=61440,wsize=65536,actimeo=1)
KMT\adlibworker@stone:~$ ls al keksrezept*
-rw-r--r- 1 KMT\adlibworker KMT\domain users 489057 Mar 19 13:08
keksrezept.jpg
rw-r--r- 1 KMT\adlibworker KMT\domain users 30254 Mar 19 13:08
keksrezept-small.jpg
KMT\adlibworker@stone:~$ pwd
/home/adlibworker
KMT\adlibworker@stone:~$ cp keksrezept* win2k-share-with-cache/ && cp
keksrezept* win2k-share-without-cache/
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666"
win2k-share-without-cache/keksrezept.jpg
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666"
win2k-share-without-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666"
win2k-share-with-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666"
win2k-share-with-cache/keksrezept.jpg

the latter hangs until it is SIGKILLed, strace'ing it does not yield
anything - same symptoms as with pyexiv2. exiv2 ist from the wheezy
repositories.

Getting to the bottom of this isn't going to be easy if this is an
issue in Samba. I had a google into Samba/Ubuntu 14.04. Gosh, there
are a lot of folks complaining about Samba4.

i am more than willing to try the procedure above again, and i am more
or less comfortable with compiling exiv2 or other binaries myself …

thank you for your time & with kind regards,
t.

Thanks for all this effort.

The build is straight forward and documented here: http://dev.exiv2.org/projects/exiv2/wiki/How_do_I_build_Exiv2_on_the_XYZ_platform

This on-liner should do the trick:

$ svn export svn://dev.exiv2.org/svn/trunk ; cd trunk ; make config ; ./configure ; make -j4 ; sudo make install ; make samples ; make tests

$ grep class ~/gnu/exiv2/trunk/src/basicio.cpp 
// class member definitions
    //! Internal Pimpl structure of class FileIo.
    class FileIo::Impl {
    }; // class FileIo::Impl
    //! Internal Pimpl structure of class MemIo.
    class MemIo::Impl {
    }; // class MemIo::Impl
            // call super class method
    //! Internal Pimpl abstract structure of class RemoteIo.
    class RemoteIo::Impl {
    }; // class RemoteIo::Impl
    //! Internal Pimpl structure of class HttpIo.
    class HttpIo::HttpImpl : public Impl  {
    }; // class HttpIo::HttpImpl
    //! Internal Pimpl structure of class RemoteIo.
    class CurlIo::CurlImpl : public Impl  {
    }; // class RemoteIo::Impl
    //! Internal Pimpl structure of class RemoteIo.
    class SshIo::SshImpl : public Impl  {
    }; // class RemoteIo::Impl

It's remarkable that you and Calvin have shown up on consecutive days talking about this. Most of the I/O code has been in service for years without anything like this being mentioned before. It'll fun to find this and say "Ah........ who would have thought ......... of course!"

I've done some work on this. No result - however progress.

1. I've persuaded smb4k to connect Linux to Windows
2. I've reproduced the hang Ubuntu <-> Windows 7 Ultimate
3. I've used exiv2/samples/iotest over Ubuntu <-> Windows 7 and Ubuntu <-> Ubuntu

samples/iotest makes two copies of a file, one with FileIO and one with MemIO. It's working fine.

1042 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ ls -alt Fonts.zip
-rwxr-xr-x 1 rmills rmills 64143832 Mar 19 20:18 Fonts.zip
1043 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ ~/gnu/exiv2/trunk/bin/iotest Fonts.zip P.zip Q.zip
1044 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ ls -alt Fonts.zip P.zip Q.zip
-rwxr-xr-x 1 rmills rmills 64143832 Mar 19 20:26 Q.zip
-rwxr-xr-x 1 rmills rmills 64143832 Mar 19 20:26 P.zip
-rwxr-xr-x 1 rmills rmills 64143832 Mar 19 20:18 Fonts.zip
1045 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ 
1045 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ md5sum Fonts.zip P.zip Q.zip
46559156b6ffb2c4777a0eaa4b039110  Fonts.zip
46559156b6ffb2c4777a0eaa4b039110  P.zip
46559156b6ffb2c4777a0eaa4b039110  Q.zip
1046 rmills@rmillsmm-kubuntu:~/smb4k/RMILLS-LAPTOP/C/temp $ 

Calvin's original script is:

for i in `ls`; do exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii New Exif comment3" $i; done

for i in `ls`; do exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii New Exif comment3" $i; sleep 1; done

I've run this from Windows7 to the Ubuntu server:

P:\home\rmills\temp\exiv2test>for %i in (*.jpg) do exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii New Exif comment3" %i

Tomorrow I will instrument and the trace all the I/O calls. The clear suspect is: exiv2+Linux-client. The Mac and Windows clients appear fine. All the servers appear fine. And we know the exiv2 file and memio classes are solid. We just keep digging for that "Ah" moment.

Incidentally, I discovered why I couldn't connect smb4k to Windows. It was smb4k. It wouldn't connect to Ubuntu (although that had been working). The /var/lib/samba/private directory was missing! Gone! Disappeared! I recreated it with smbpasswd and everything worked.

Guys, if you have any ideas - please share. I've added Calvin as a watcher for this issue and intend to discuss both 1042 and 1043 in this thread.

Ok, I've tested the following:

XP SMB client <-> Ubuntu 14.04.2 LTS on just over 4000 files with a similar windows batch file.
2 of the 3 binaries you made available (trunk and .23)
not reproducible.

On Ubuntu 14.04.2 LTS NFS Client to NFS Server on just over 4000 file
My compiled trunk and standard ubuntu version "exiv2 -V
exiv2 0.23 001700 (64 bit build)"
not reproducible.

So this definitely points to a CIFS client issue on Ubuntu.

Unfortunately, I'm off to Germany on Sunday for WHD and then some leave, so it's going to be 2 weeks before I can start getting into that.

In the meantime - maybe a readme in the release notes, with a recommendation on nfs over cifs? When I get back I can also try an earlier ubuntu version, assuming it has an older cifs client.

hi robin,

thank you for having a look! i can confirm that your patch resolves the issue:

KMT\adlibworker@stone:~$ which exiv2
/usr/local/bin/exiv2
KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -V | head -n5
exiv2 0.24 001800 (64 bit build)
Copyright (C) 2004-2013 Andreas Huggel.

This program is free software; you can redistribute it and/or
modify it under the terms of the GNU General Public License
KMT\adlibworker@stone:~$ cp keksrezept* win2k-share-without-cache/
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666" win2k-share-without-cache/keksrezept.jpg
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666" win2k-share-without-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$ cp keksrezept* win2k-share-with-cache/
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666" win2k-share-with-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$ exiv2 -M"set Exif.Photo.PixelXDimension 666" win2k-share-with-cache/keksrezept.jpg
KMT\adlibworker@stone:~$

so, not using mmap'ed temporary files does not trigger the bug … interesting. i also tried with this:

thoralf@tfbb:~/Baustelle/exiv2/trunk$ diff ub src/basicio.cpp{.orig,}
-- src/basicio.cpp.orig 2015-03-23 12:40:37.672506212 0100
++ src/basicio.cpp 2015-03-23 13:24:45.273634955 +0100
@ -584,7 +584,8 @
// buffer, which is a workaround to ensure that the links don't get broken.

// #1042 and #1043.  Always use a FileIo object to avoid Samba/Ubuntu/cifs error
-        ret = 911;
+        // try with msync()
+        // ret = 911;
         if (ret != 0 || (buf.st_size > 1048576 && nlink == 1)) {
             pid_t pid = ::getpid();
             std::auto_ptr<FileIo> fileIo;
@ -1177,6 +1178,7 @
     MemIo::~MemIo()
     {
         if (p_->isMalloced_) {
+            msync(p_, p_->size_, MS_ASYNC);
             std::free(p_->data_);
         }
         delete p_;
thoralf@tfbb:~/Baustelle/exiv2/trunk$

… which also seems to work. however, i do not speak c(++) and wouldn't trust this code to not set my house on fire or eat my babies. fortunately, i have neither :-)
i'm also not sure if the unconditional msync doesn't defeat the purpose of using mmap'ed files in the first place.

Unfortunately, I'm off to Germany on Sunday for WHD and then some leave

say hello if you are in berlin - beers will be on me :-)

thank you very much & with kind regards,
t.

Thanks You Thoral for your suggestion about msync(). I doubt if it would eat your (non-existing) babies.

In view of Thomas observation that the 911 fix causes #812 to resurface on Windows, I'll have to read the code with more care to understand this more deeply. I did ask Andreas to review this fix as I believe he wrote the code. I see he was involved in #812 which involves erroneously "emptying" files which have multiple hard links.

My thoughts are that I should:

1. Study #812 to understand what's going on
2. Add to test/bugfixes-test.sh to detect #812
3. Perhaps MemIo can never be used to rewrite a file nlinks > 1 on any platform

Specifically, about msync():

1. munmap()/mmap() may be portable and have the same effect as msync()
2. munmap()/mmap() may cause the MemIo encapsulated pointer to change (hopefully without side-effects)

Ah...... Its often tough to deal with other folks code. However getting to the bottom of these matters can be satisfying. For sure, we've made considerable progress in one week.

If you get to meet Calvin, enjoy your beer. If Thomas can also join you, so much the merrier. "Cheers" from Merry England.

#812 is about destroying hard links after writing metadata to file. There is a link to a 3 year old discussion inside #812 but you don't need to follow it all through. Just see the comment above your 911-commit in basicio.cpp, lines 583 and 584:

// use a memory buffer. I.e., files with hard links always use a memory
// buffer, which is a workaround to ensure that the links don't get broken.

This does no longer hold after r3627. Thus hot-fixing this ticket broke another one.

One more sentence about constructive input: If you read my previous comment again you'll see that I pointed to msync as a possible solution fixing this one and keeping #812 closed.

One more thought about #812. I guess it's not purely related to Windows and should also apply to Linux. And of course, you are right about adding the missing test case.

Thank you, Robin.

Thomas

I think there has been a misunderstanding. You have been very helpful by pointing out that the 911 fix breaks the fix to #812. However increasing the priority of the bug amounts to you shouting at me. Please don't do that.

You are probably correct that protecting the msync() call with #if HAVE_MSYNC .. #endif is a good fix. I feel I must add an #812 detector to the test suite (in case it is erroneously reintroduced). And I will need to add an HAVE_MSYNC detector to both the autotools and CMake build environments. We know it's not available on MSVC. I don't know its status on Cygwin and MInGW.

I do appreciate your input and help. I will spend more time on this.

Robin

Oh, I've thought of something else. I'll confess: "I didn't read the comment about the hard links!". Or, I glanced at it without realising the implication. I was so focused on the size condition that I thought "what's size go to do with it?" (as Tina Turner once sang).

So, I thought res = 911; makes it very clear that we're always gonna head down the FileIo route and never risk MemIo. And then I thought "I'll dump this on Andreas to review". However he's rather dormant on the project at the moment and we're going to have to live without him.

The good news is that the team of Thomas, Thoralf, Calvin and Robin seem to be pulling together to fix this. Well Done Everybody. Sounds like Thoralf is offering to buy everybody beer in Berlin one night this week. It's my Wedding Anniversary on Friday (41 happy years). I'll toast your health with a glass of English Bitter if we've nailed this issue by the end of the week.

Just checked: msync is in sys/mman.h of Cygwin. Mingw should be also positive. Can check tomorrow.

Thanks, Thomas.

I expect them to be there. However, I'm updating configure.ac to test for itself and set HAVE_MSYNC. You never know, there might be an old Cygwin (or worse, a future Cygwin) that doesn't include it. Or when compiling with Clang. Or an unsupported platform such as Sun or AIX which may, or may not, have msync(). Better to leave autotools/CMake to test-link the the function to be sure it's on the build platform. And I'll update src/version.cpp which not reports those flags when your run $ exiv2 --verbose --version

I'm hoping to fix this before going to bed. Adding something to the test suite will be done later. I won't mark this issue "Resolved" until I have something in the test suite to detect #812.

There are already detectors for EXV_HAVE_SYS_MMAN_H, EXV_HAVE_MMAP and EXV_HAVE_MUNMAP.

Unfortunately I cannot answer Thoralfs question if unconditional msync doesn't defeat the purpose of using mmap'ed files.

If it proves to be the solution what about moving msync from dtor of MemIO to MemIo::munmap() or MemIo::close() which are a dummy function right now?

I'm not sure either about Thoralf's question about msync defeating using mmap'ed file.

I'm focused on adding msync() and a detector. Exactly where we put the msync() call isn't too important - it should be protected by #if HAVE_MSYNC ... #endif. Maybe I will should add a method MemIo.msync() and hide the detector there.

However .... it's getting late in Germany ... you go to bed and I'll finish the autotools/CMake/version.cpp code and test it tonight. I'll manually test nlinks > 1 on the Mac. Adding to test/bugfixes-test.sh isn't getting done tonight.

OK, guys. Fix submitted. r3630

Thanks to everybody for getting involved on this - especially Thomas. Well done, Thomas: you pushed me to realise my error. Thanks.

I hope we're really done on this. I've checked on Mac and Linux that we're not destroying files with multiple hard links.

$ exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii New Exif comment3" R.jpg
File 1/1: R.jpg
Set Exif.Photo.UserComment "charset=Ascii New Exif comment3" (Comment)
$ ln R.jpg Q.jpg
$ exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii changed the comment" R.jpg
File 1/1: R.jpg
Set Exif.Photo.UserComment "charset=Ascii changed the comment" (Comment)
$ exiv2 -pa -g UserComment R.jpg
Exif.Photo.UserComment                       Undefined  27  changed the comment
$ exiv2 -pa -g UserComment Q.jpg
Exif.Photo.UserComment                       Undefined  27  changed the comment

And, we got lucky. The macro MS_SYNC is a companion of msync(), so I didn't need to update the build environment. I'll check the build server tomorrow to be sure that this has build OK on all platforms (perhaps MSVC has a different MS_SYNC macro!).

If nobody says "not fixed" then I'll add something to ./test/bugfixes-test.sh later this week and update this issue to resolved.

Thank you, Robin.

Just a quick comment for now: I guess there is once call of msync() missing in function MemIo::transfer in case memIo!=this.

Do you mean:

    void MemIo::transfer(BasicIo& src)
    {
        MemIo *memIo = dynamic_cast<MemIo*>(&src);
        if (memIo) {
            // Optimization if src is another instance of MemIo
            if (true == p_->isMalloced_) {
                std::free(p_->data_);
            }
            p_->idx_ = 0;
            p_->data_ = memIo->p_->data_;
            p_->size_ = memIo->p_->size_;
            p_->isMalloced_ = memIo->p_->isMalloced_;
            memIo->p_->idx_ = 0;
            memIo->p_->data_ = 0;
            memIo->p_->size_ = 0;
            memIo->p_->isMalloced_ = false;
        }
        else {
            // Generic reopen to reset position to start
            if (src.open() != 0) {
                throw Error(9, src.path(), strError());
            }
            p_->idx_ = 0;
            write(src);
            src.close();
        }
        if (error() || src.error()) throw Error(19, strError());
    }

...
        if (memIo) {
            // Optimization if src is another instance of MemIo
            if (true == p_->isMalloced_) {
                msync();
                std::free(p_->data_);
            }
...

• MemIo::munmap() could return the return value of MemIo::msycn().
• No need to have MemIo::msycn() virtual. Could be even private.

Robin Mills wrote:

Do you mean:[...]
Should be:[...]

Yes, exactly.

Important question is also if msync() must be called before FileIO::munmap is called. According to msync documentation ist should be called. Unfortunately I am not a Linux API expert.

It was confusing to me that msync doc refers to mmap/munmap which is not used in MemIO but FileIO.

This one is interesting: http://stackoverflow.com/questions/5902629/mmap-msync-and-linux-process-termination

msync is used case of mmap/munmap = FileIO. For MemIO class we could try fsync.

I'm a little lost here, Thomas. Where is this "if msync() must be called before FileIO::munmap is called" ?

I call the msync() method in MemIo:munmap() in r3630. And MemIo::msync() could be declared private and does not need to be virtual. However I don't see anything incorrect in what's been submitted.

And Thomas. I have another life. I can't endlessly explore and test every possibility. It appears to be building and working fine on all supported platforms. I have Daniel moaning at me about CMake/MSVC. Gilles discussing the webp GSoC 2015 project. Alan wants something about Lightroom Categories. And I have 14 other oustanding issues to deal with for v0.25. Oh, and more jenkins/build-server stuff to finish. And you how many volunteers do I have for all those tasks.

And I'm rather puzzled that the Windows Memory Mapping appears to be used in the FileIo class. I didn't write it and don't have time to deal with all of this.

Thomas. Please provide a patch with all your suggested changes. I'll review and submit. However I can't endlessly work on this. I have other things to get on with - including my life!

I'm a little lost here, Thomas. Where is this "if msync() must be called before FileIO::munmap is called" ?

This ought to be a question. It is taken from here: http://man7.org/linux/man-pages/man2/msync.2.html

msync() flushes changes made to the in-core copy of a file that was
mapped into memory using mmap(2) back to the filesystem. Without use
of this call there is no guarantee that changes are written back
before munmap(2) is called.

Currently we don't do it in FileIO. That's why I asked the Linux API experts.

And I'm rather puzzled that the Windows Memory Mapping appears to be used in the FileIo class. I didn't write it and don't have time to deal with all of this.

Yes, for sure it is there. This is the usual Win-API programming compared to Linux API mmap/munmap. No need to bother with this.

Please provide a patch with all your suggested changes. I'll review and submit. However I can't endlessly work on this. I have other things to get on with - including my life!

I can try. But I cannot test if it fixes this ticket.

hi,

Thomas Beutlich wrote:

[…] But I cannot test if it fixes this ticket.

i have a working build environment for linux amd64 here, and i am more than willing to keep it around for a little while. i didn't use the test suite yet, but am fairly confident to be able to figure it out … i won't be able to check robins latest checkin today, but will do so tomorrow.

Robin Mills wrote:

Beers in Berlin on Friday Night - 2015-03-27 - to toast Exiv2 and my 41st Wedding Anniversary.

sounds good to me, my offer "free beer to anyone involved" still holds :-) for the berliners, i'd suggest the bierkombinat kreuzberg, ~8 pm? robin, we'll have a toast on you and you anniversary.

with kind regards,
t.

Thomas.

The build server builds and tests the code when it's submitted. http://exiv2.dyndns.org:8080

When you give me your patch, I'll review and submit it. If it builds and passes the test suite on the build server, we're done! However I can't spend any more time on this.

Thoralf

Thanks for your input. If Thomas comes for beer on Friday, don't let him discuss this issue (or his other contributions) all night. Drown him in beer!

Robin

Patch submitted. r3633. Thank You Thomas for providing the patch.

I will keep myself as the "Assignee" on this issue and check the build server tomorrow. If everything's fine (which I expect it to be), I'll mark this "Resolved" and change the Assignee to Thomas to recognise his important contribution. However this has been a real team effort to fix this. Thank You everybody.

Yes, builds should likely be fine. But tests on this issue are more interesting. Thoralf can help here and should be the one credited for adding msync.

hi,

i checked r3633. unfortunately, it exhibits the same symptoms as versions before r3627 - it hangs on large files, does work fine while being strace'd etc.
i also double-checked with a fresh checkout of r3627, which still works fine.

sorry for the bad news, please don't kill the messenger …

with kind regards,
t.

Thanks for testing and reporting this. We have to know about this sooner or later. Much better to know sooner.

Thoralf, can you please repeat the test where you replace MS_SYNC by MS_ASYNC in basicio.cpp. This is the only difference I can think about.

hi,

Thomas Beutlich wrote:

Thoralf, can you please repeat the test where you replace MS_SYNC by MS_ASYNC in basicio.cpp. This is the only difference I can think about.

i did - same issue, hangs as well. i also did another test with r3627 and the msync()-patch applied, which failed in the same way this time :-(

so msync seems to have been a red herring, and i feel rather stupid now.

sorry,
t.

Thanks for testing. Very strange observations.

I will try to debug it once more to see how FileIO and MemIO work. I won't be able to do it today.

Gentlemen:

Are you sure this is a problem. It seems to be working OK for me. Here's the setup:

• 7mb JPG - with/without links.
• client is Ubuntu (machine = rmillsmbp-kubuntu)
• directory is mounted with cifs on rmillsmm-kubuntu (running 14.10)
• exiv2 built on head of trunk. basicio.cpp is r3635

1. Run a loop of 100 write/reads on foo.jpg. Result OK
2. hard-link P.jpg and Q.jpg to foo.jpg
3. Run a loop of 10 write/reads on foo.jpg. Result OK

Here's the evidence:

$ uname -a
Linux rmillsmbp-kubuntu 3.13.0-46-generic #79-Ubuntu SMP Tue Mar 10 20:06:50 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

$ svn info ~/gnu/exiv2/trunk/src/basicio.cpp | grep Rev
Revision: 3644
Last Changed Rev: 3635

$ exiv2 -vV -g svn -g date -g time
exiv2 0.24 001800 (64 bit build)
date=Mar 25 2015
time=22:39:42
svn=3644
have_gmtime_r=1
have_timegm=1

$ df --print-type .
Filesystem                Type 1K-blocks     Used Available Use% Mounted on
//RMILLSMM-KUBUNTU/rmills cifs  63859580 12738888  47853776  22% /home/rmills/smb4k/RMILLSMM-KUBUNTU/rmills

$ ls -alt foo.jpg
-rw-r--r-- 1 rmills rmills 7169528 Mar 25 22:42 foo.jpg

$ for i in {1..100}; do exiv2 -M"set Exif.Photo.UserComment hello $i" foo.jpg ; exiv2 -pa -g UserComment foo.jpg; done
Exif.Photo.UserComment                       Undefined  15  hello 1
Exif.Photo.UserComment                       Undefined  15  hello 2
...
Exif.Photo.UserComment                       Undefined  16  hello 99
Exif.Photo.UserComment                       Undefined  17  hello 100

$ ln foo.jpg P.jpg
$ ln foo.jpg Q.jpg

$ for i in {200..210}; do exiv2 -M"set Exif.Photo.UserComment hello $i" foo.jpg ; exiv2 -pa -g UserComment *.jpg; done     
foo.jpg               Exif.Photo.UserComment                       Undefined  17  hello 200
P.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 200
Q.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 200
foo.jpg               Exif.Photo.UserComment                       Undefined  17  hello 201
P.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 201
Q.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 201
...
foo.jpg               Exif.Photo.UserComment                       Undefined  17  hello 210
P.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 210
Q.jpg                 Exif.Photo.UserComment                       Undefined  17  hello 210
$

hi,

i am able to use exiv2 / libexiv2 with mount.cifs "cache=none" option, which is a more or less workable situation here …

i also realise that mounting smb shares form a win2k8 server might not really be a commonplace endeavour. and finally, i am willing to grant shell access to the box where this issue persists to anyone who wants to investigate this issue further.

short of other ideas (and still not being the c++ guy), i sprinkled debug statements all over the place (i've been using r3637):

thoralf@tfbb:~/Baustelle/exiv2/trunk$ diff -u1 -b src/actions.cpp{.orig,} 
--- src/actions.cpp.orig        2015-03-27 17:19:02.597381466 +0100
+++ src/actions.cpp     2015-03-27 16:29:12.098552387 +0100
@@ -1215,2 +1215,3 @@
     try {
+        std::cout << "modify_run\n";
         if (!Exiv2::fileExists(path, true)) {
@@ -1248,2 +1249,3 @@
     {
+        std::cout << "modify_apply\n";
         if (!Params::instance().jpegComment_.empty()) {
@@ -1290,2 +1292,3 @@
     {
+        std::cout << "modify_addmeta\n";
         if (Params::instance().verbose_) {
@@ -1326,2 +1329,3 @@
     {
+        std::cout << "modify_setmeta\n";
         if (Params::instance().verbose_) {
@@ -1398,2 +1402,3 @@
     {
+        std::cout << "modify_delmeta\n";
         if (Params::instance().verbose_) {
@@ -1430,2 +1435,3 @@
     {
+        std::cout << "modify_regname\n";
         if (Params::instance().verbose_) {
@@ -1453,2 +1459,3 @@
     try {
+        std::cout << "adjust_run\n";
         adjustment_      = Params::instance().adjustment_;
thoralf@tfbb:~/Baustelle/exiv2/trunk$
thoralf@tfbb:~/Baustelle/exiv2/trunk$ diff -u2 -b src/basicio.cpp{.orig,} 
--- src/basicio.cpp.orig        2015-03-27 10:47:20.112838974 +0100
+++ src/basicio.cpp     2015-03-27 16:58:35.687297553 +0100
@@ -182,4 +182,5 @@
     int FileIo::Impl::switchMode(OpMode opMode)
     {
+        // std::cout << "FileIo::Impl::switchMode here\n";
         assert(fp_ != 0);
         if (opMode_ == opMode) return 0;
@@ -369,4 +370,5 @@
     int FileIo::munmap()
     {
+        std::cout << "fileio::munmap here\n";
         int rc = 0;
         if (p_->pMappedArea_ != 0) {
@@ -403,4 +405,5 @@
     byte* FileIo::mmap(bool isWriteable)
     {
+        std::cout << "fileio::mmap here\n";
         assert(p_->fp_ != 0);
         if (munmap() != 0) {
@@ -572,4 +575,5 @@
         BasicIo::AutoPtr basicIo;

+        std::cout << "hi there, basicio here\n";
         Impl::StructStat buf;
         int ret = p_->stat(buf);
@@ -587,4 +591,5 @@
         // ret = 911;
         if (ret != 0 || (buf.st_size > 1048576 && nlink == 1)) {
+            std::cout << "doing file-io\n";
             pid_t pid = ::getpid();
             std::auto_ptr<FileIo> fileIo;
@@ -615,4 +620,5 @@
         }
         else {
+            std::cout << "doing mem-io\n";
             basicIo.reset(new MemIo);
         }
@@ -653,4 +659,5 @@
     void FileIo::transfer(BasicIo& src)
     {
+        std::cout << "FileIo::transfer here\n";
         const bool wasOpen = (p_->fp_ != 0);
         const std::string lastMode(p_->openMode_);
thoralf@tfbb:~/Baustelle/exiv2/trunk$ 

KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept-small.jpg 
File 1/1: win2k-share-with-cache/keksrezept-small.jpg
modify_run
fileio::munmap here
fileio::munmap here
fileio::munmap here
fileio::munmap here
fileio::munmap here
modify_apply
modify_setmeta
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap here
hi there, basicio here
doing mem-io
Info: Write strategy: Intrusive
fileio::munmap here
FileIo::transfer here
fileio::munmap here
fileio::munmap here
fileio::munmap here
KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg 
File 1/1: win2k-share-with-cache/keksrezept.jpg
modify_run
fileio::munmap here
fileio::munmap here
fileio::munmap here
fileio::munmap here
fileio::munmap here
modify_apply
modify_setmeta
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap here
hi there, basicio here
doing mem-io
Info: Write strategy: Intrusive
fileio::munmap here
FileIo::transfer here
fileio::munmap here
[ … hang occurs right here ]
^C
KMT\adlibworker@stone:~$ 

• the hanging process can be terminated by pressing ^C now
• fileio::mmap doesn't seem to be called at all
• commenting in the std::cout << "FileIo::Impl::switchMode here\n";-statement makes the code work for both my testfiles (as does strace'ing the original code, which might be related)

with kind regards,
t.

hi,

s/3637/3627 - time for beer, and bon appetit to the curry connoisseurs as well :-)

t.

Very strange that FileIo::mmap() is never called.

Can you please instrument further in FileIo::munmap(), especially before and after ::munmap()? I guess that it does not return in case it hangs.

See also my comment:28 again: Important question is also if ::msync() must be called before FileIO::munmap() is called? Can you add it and test once more?

hi thomas,

Thomas Beutlich wrote:

See also my comment:28 again: Important question is also if ::msync() must be called before FileIO::munmap() is called? Can you add it and test once more?

where exactly do you want me to put it?

with kind regards,
t.

Gentlemen

I am now on vacation. We have friends here from California. I will be checking email while on the road, however my time for working is limited. They leave on April 10.

I have requested write access to the repository for Thomas and I wish you well in getting to the bottom of this matter. To avoid debugging this on the trunk, I will set up a new branch "memio-investigation" and monitor in with Jenkins. http://exiv2.dyndns.org:8080/

Robin

I just started to debug (on Win though).

Yes, I also disgagree on moving on too early. Behaviour shall be identical for small and large files!

Thank you both Thoralf and Thomas for getting involved with this. I'm exhausted and need a break.

OK, FileIo::munmap() actually does nothing since for jpg files there never is FileIo::mmap() called. Thus we do not need to further concentrate on this. Telling from Thoralf logs I propose to further concentrate on function FileIo::transfer().

• in FileIo::munmap() change the first line to

• FileIo::close() replace by

@
int FileIo::close() {
int rc = 0;
if (munmap() != 0) rc = 2;
if (p_->fp_ != 0) {
std::cout << "before close\n";
if (std::fclose(p_->fp_) != 0) rc |= 1;
std::cout << "after close\n";
p_->fp_= 0;
}
return rc;
}
@

• and FileIo::open() replace by
  @
  int FileIo::open(const std::string& mode) {
  close();
  p_->openMode_ = mode;
  p_->opMode_ = Impl::opSeek;
  #ifdef EXV_UNICODE_PATH
  if (p_->wpMode_ == Impl::wpUnicode) {
  std::cout << "before open\n";
  p_->fp_ = ::_wfopen(wpath().c_str(), s2ws(mode).c_str());
  std::cout << "after open\n";
  }
  else
  #endif {
  std::cout << "before open\n";
  p_->fp_ = ::fopen(path().c_str(), mode.c_str());
  std::cout << "after open\n";
  }
  if (!p_->fp_) return 1;
  return 0;
  }
  @

hi,

robin: thank you for your cooperation :-)
thomas … thank you for having a look. small file:

KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept-small.jpg 
File 1/1: win2k-share-with-cache/keksrezept-small.jpg
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before open
after open
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
before open
after open
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before open
after open
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before open
after open
Info: Write strategy: Intrusive
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before open
after open
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
fileio::munmap 0x14d3a50 win2k-share-with-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$ 

KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg 
File 1/1: win2k-share-with-cache/keksrezept.jpg
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
before open
after open
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
Info: Write strategy: Intrusive
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
fileio::munmap 0x2004a50 win2k-share-with-cache/keksrezept.jpg
KMT\adlibworker@stone:~$ 

KMT\adlibworker@stone:~$ /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg 
File 1/1: win2k-share-with-cache/keksrezept.jpg
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
before open
after open
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
Info: Write strategy: Non-intrusive
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before close
after close
fileio::munmap 0x1705a50 win2k-share-with-cache/keksrezept.jpg
before open
after open

[ … hang ]

thank you & with kind regards,
t.

btw: please send me an openvpn certificate request if you need access to the machine …

Can you try ro reproduce the hang with intrusive write strategy. This is different in the last two logs of the big file. Maybe you need to set other value than 66666.

On Win it looks completely different where a temp file is opened.

• Compile exiv2 with -g
• In shell type: gdb exiv2
• Then type in gdb shell: run -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
• When it hangs type: where

hi thomas,

Thomas Beutlich wrote:

Can you try ro reproduce the hang with intrusive write strategy. This is different in the last two logs of the big file. Maybe you need to set other value than 66666.

seems to be hit and miss … sometimes exiv2 does not hang when intrusive is being used, most of the time it does - the successful execution in #57 seems to be a rare exception. when it succeeds to write the big file, it resorts to non-intrusive on the next run, which then also hangs most of the time.

Can you please debug where it hangs with gdb?

• Compile exiv2 with -g
• In shell type: gdb exiv2
• Then type in gdb shell: run -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
• When it hangs type: where

i am not able to ^c back to gdb once exiv2 hangs:

KMT\adlibworker@stone:~$ gdb /usr/local/bin/exiv2
GNU gdb (GDB) 7.4.1-debian
Copyright (C) 2012 Free Software Foundation, Inc.
[ bla … ]
Reading symbols from /usr/local/bin/exiv2...done.
(gdb) run -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
Starting program: /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
File 1/1: win2k-share-with-cache/keksrezept.jpg
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before open
[ … like above]
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before open
after open
[ … hang occurs here ]
^C
where
^C
where
^C^C^C^C

where
bt

… i'll try to play around with breakpoints.

with kind regards,
t.

Thanks for testing. FileIO::transfer() should be a good candicate to play with.

hi thomas,

the hang occurs in FileIo::transfer's write(src)-line - this is #896 in the non-instrumented source.

KMT\adlibworker@stone:~$ gdb /usr/local/bin/exiv2 
[…]
(gdb) # this is the beginning of void FileIo::transfer(BasicIo& src)
(gdb) break basicio.cpp:690
(gdb) run -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
Starting program: /usr/local/bin/exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" win2k-share-with-cache/keksrezept.jpg
File 1/1: win2k-share-with-cache/keksrezept.jpg
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before open
after open
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
[…]
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before open
after open
Info: Write strategy: Intrusive
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before close
after close
FileIo::transfer here

Breakpoint 2, Exiv2::FileIo::transfer (this=0x62aa30, src=...) at basicio.cpp:691
warning: Source file is more recent than executable.
691             const bool wasOpen = (p_->fp_ != 0);
(gdb) n
692             const std::string lastMode(p_->openMode_);
(gdb) n
691             const bool wasOpen = (p_->fp_ != 0);
(gdb) n
692             const std::string lastMode(p_->openMode_);
(gdb) n
691             const bool wasOpen = (p_->fp_ != 0);
(gdb) n
692             const std::string lastMode(p_->openMode_);
(gdb) n
694             FileIo *fileIo = dynamic_cast<FileIo*>(&src);
(gdb) n
695             if (fileIo) {
(gdb) n
694             FileIo *fileIo = dynamic_cast<FileIo*>(&src);
(gdb) n
695             if (fileIo) {
(gdb) n
913                 if (open("w+b") != 0) {
(gdb) n
fileio::munmap 0x62aa50 win2k-share-with-cache/keksrezept.jpg
before open
after open
924                 if (src.open() != 0) {
(gdb) n
935                 write(src);
(gdb) n
[ … hang]

with kind regards,
t.

hi,

a bit more detail:

677                 writeTotal += writeCount = (long)std::fwrite(buf, 1, readCount, p_->fp_);
(gdb) n
678                 if (writeCount != readCount) {
(gdb) n
676             while ((readCount = src.read(buf, sizeof(buf)))) {
(gdb) n
677                 writeTotal += writeCount = (long)std::fwrite(buf, 1, readCount, p_->fp_);
(gdb) n
678                 if (writeCount != readCount) {
(gdb) n
676             while ((readCount = src.read(buf, sizeof(buf)))) {
(gdb) n
677                 writeTotal += writeCount = (long)std::fwrite(buf, 1, readCount, p_->fp_);
(gdb) n
n
[… hang]

with kind regards,
t.

hi,

amending the write-loop in long FileIo::write(BasicIo& src) like this:

        while ((readCount = src.read(buf, sizeof(buf)))) {
            std::cout << "write loop … rc: " << readCount << " wc: " << writeCount << " wt: " << writeTotal << "\n";
            writeTotal += writeCount = (long)std::fwrite(buf, 1, readCount, p_->fp_);
            if (writeCount != readCount) {
                // try to reset back to where write stopped
                src.seek(writeCount-readCount, BasicIo::cur);
                break;
            }
        }

FileIo::transfer here
fileio::munmap 0xaf0a50 win2k-share-with-cache/keksrezept.jpg
before open
after open
in fileio::write …
write loop … rc: 4096 wc: 0 wt: 0
write loop … rc: 4096 wc: 4096 wt: 4096
write loop … rc: 4096 wc: 4096 wt: 8192
write loop … rc: 4096 wc: 4096 wt: 12288
write loop … rc: 4096 wc: 4096 wt: 16384
[ … hang]

FileIo::transfer here
fileio::munmap 0x1ecea50 win2k-share-with-cache/keksrezept-small.jpg
before open
after open
in fileio::write …
write loop … rc: 4096 wc: 0 wt: 0
write loop … rc: 4096 wc: 4096 wt: 4096
write loop … rc: 4096 wc: 4096 wt: 8192
write loop … rc: 4096 wc: 4096 wt: 12288
write loop … rc: 4096 wc: 4096 wt: 16384
write loop … rc: 4096 wc: 4096 wt: 20480
write loop … rc: 4096 wc: 4096 wt: 24576
write loop … rc: 1648 wc: 4096 wt: 28672
fileio::munmap 0x1ecea50 win2k-share-with-cache/keksrezept-small.jpg
before close
after close
fileio::munmap 0x1ecea50 win2k-share-with-cache/keksrezept-small.jpg
KMT\adlibworker@stone:~$

with kind regards,
t.

Guys

Can I encourage you to look at samples/iotest.cpp. It's effectively a file copy program which uses both FileIO and MemIO in basicio.cpp. It's used by our test suite - however it's worth playing with this a little on large and small files. It's easy to understand it.

Thanks for doing this work.

Robin

To give an expression how it looks on Win:

C:\Projekte\wdx_jpg_comment_src\exiv2\msvc\exiv2\Debug>exiv2 -v -Q d -M"set Exif.Photo.PixelYDimension 66666" keksrezept.jpg
File 1/1: keksrezept.jpg
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before open
after open
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
before open
after open
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before open
after open
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
Set Exif.Photo.PixelYDimension "66666" (Long)
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before open
after open
fileio::munmap 007BB9B8 keksrezept.jpg2056
 . common block
   . iswriteable: 0
 . return value: 0
before open
after open
Info: Write strategy: Non-intrusive
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
fileio::munmap 007BB9B8 keksrezept.jpg2056
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before open
after open
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
before close
after close
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0
fileio::munmap 007BB9B8 keksrezept.jpg2056
 . common block
   . iswriteable: 0
 . return value: 0
fileio::munmap 003A7370 keksrezept.jpg
 . common block
   . iswriteable: 0
 . return value: 0

Sorry, forget about my last comment. We are doing MemIo here which is case for large files having hard links.

• Increase buffer size to some greater value. For test you could also try to set the exact file size (or some greater value) as buffer size such that there is only one call to std::fwrite().
• Call setvbuf(p_->fp_, NULL, _IONBF, 0); anywhere before the while loop.
• Call setvbuf(p_->fp_, (char*)buf, _IOFBF, 4096); anywhere before the while loop.
• Call setvbuf(p_->fp_, NULL, _IOFBF, 4096); anywhere before the while loop.
• Call fflush(p_->fp_); after each call to std::fwrite().
• Swap second and third element of std::fwrite()@to @std::fwrite(buf, readCount, 1, p_->fp_). Should not matter.

hi there,

just a quick note: i'll be on vacation next week and won't show up here. i will definitely have a look at iotest.cpp and thomas' suggestions afterwards …

thank you & with kind regards,
t.

Have a nice break, Thoralf. We'll talk when you (and I) get back to work.

Robin

Hi Guys,

got back from my break.

svn checkout svn://dev.exiv2.org/svn/trunk
make config
cd trunk/
make config
./configure

and yay!

for i in `ls`; do ~/exiv2-trunk/trunk/bin/exiv2 -v -M"set Exif.Photo.UserComment charset=Ascii New Exif comment2" $i; done

runs through, several times with no truncations. (Ubuntu 14.04.2 LTS client & server)

did a 'make install' and loaded up shotwell and it no hangs on my 50k library (although to be fair only a portion of them were due to have comments rewritten).

So, from my perspective all looks good.

Any pointers on getting this fix into the Ubuntu project? (I'm not fortunate enough to be involved in these things regularly so know the procedures so pointers would help).

Many thanks!

--Calvin

Calvin

How nice to hear from you. And to hear your good news.

Thoralf and Thomas are continuing to work on this matter. There's no smoke without fire and Thoralf can see smoke on Windows Server 2008. So T & T are going to continue to work on this and that enables me to pursue other matters.

Hope you've enjoyed your trip.

Robin

thoralf schulze: Do you have any news on my above suggestions? Obviously you are the only one left who can reproduce the problem.

hi thomas, hi all,

… sorry for taking so long, i am drowning in work. i was just about to try thomas' suggestions and cannot reproduce the issue anymore. long story short: about a week ago, we've had to reboot the machine that exhibited the symptoms - it all works fine now, sorry for making demands on your time.

a bit more nitty-gritty: the machine needed a reboot due to a bunch of more or less critical updates accumulating. among them was a kernel update¹ - skimming through the changelogs, i found these notes:
- cifs: fix deadlock in cifs_ioctl_clone()
- cifs: Complete oplock break jobs before closing file handle (regression in 3.15)
… both issues were fixed in 3.16.7-ckt7-1. i suspect that the latter patch fixes our issue, please see http://permalink.gmane.org/gmane.linux.kernel.stable/123919 for details.

i've let my testcase run for a while now, with both my butchered version of exiv2 and the one from the debian repositories … they did run for 1000+ iterations now, without any hiccups.

i guess we are all set now - please feel free to close this issue. "beers in berlin for anyone involved" is still (and will always be) a standing offer.

thank you very much for your time & with kind regards,
t.

¹ - linux-image-3.16.0-0.bpo.4-amd64:amd64 3.16.7-ckt4-3~bpo70+1 to 3.16.7-ckt7-1~bpo70+1

Good news. Thanks.

Will check again which commits were done here and if they were necessary.

@Robin: i think you can delete the svn branch you offered for testing.

Good thinking, Thomas. Can you review changes and tidy up if necessary. I've removed the memo-investigation branch from SVN and removed the branch monitor from Jenkins.

I revisited the commits bound to this issue, esp. r3630. I do no think that msync is necessary for MemIo. According to http://man7.org/linux/man-pages/man2/msync.2.html msync is only useful for memory mapped files which is not the case in MemIo. I would rather see it in FileIo where there are the memory mapped files. Thus my questions in #1043-28 (28) and #1043-29 (29) are still not answered.

Robin, should I open a new issue for this open questions?

Thomas:

I think #1043 (and his buddy #1042) are resolved. I'm happy if you log a new issue about msync().

As we're very close to v0.25, I'd like to delay any work on this new issue. If you believe this is an essential fix for v0.25, could you associate the new issue with #1071 which is the 'Umbrella' for v0.25.

If this new issue is not essential for v0.25, please target it at v0.26.