<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">Am 03.04.2013 00:01, schrieb Albert
      Astals Cid:<br>
    </div>
    <blockquote cite="mid:9416343.zB127GlF5Q@xps" type="cite">
      <pre wrap="">El Dimarts, 2 d'abril de 2013, a les 10:36:53, Thomas Freitag va escriure:
</pre>
      <blockquote type="cite">
        <pre wrap="">Am 02.04.2013 09:22, schrieb Thomas Freitag:
</pre>
        <blockquote type="cite">
          <pre wrap="">Hi Albert!

Am 27.03.2013 12:29, schrieb Thomas Freitag:
</pre>
          <blockquote type="cite">
            <pre wrap="">Sorry, pushed the wrong button, here my answer to the list:

Am 27.03.2013 11:41, schrieb Albert Astals Cid:
</pre>
            <blockquote type="cite">
              <pre wrap="">Why do we pass around the recursion integer around?
</pre>
            </blockquote>
            <pre wrap="">
No idea: The recursive integer wasn't introduced by the thread safe
patch, it was already there. I just used it and extend some functions
which incorrectly ignore that parameter instead of passing it to the
called functions. Probably I missed some more than only the one in
DCTStream::DCTStream.
</pre>
          </blockquote>
          <pre wrap="">
I just ran a regression test over my PDF files, but I didn't get any
dead locks. So if You want me to solve that problem, You should
provide me that PDF. (Regardless if You prefer to use recursive mutex:
the endless loop detection need to pass the recursive integer
everywhere!)
</pre>
        </blockquote>
        <pre wrap="">
Attached a patch which corrects it. I made it in my sleep, 'cause the
dead lock was probably caused by my patch for bug 61994 (even if that
PDF doesn't cause any dead locks). This time I also respect the
DCTStream usage if LIBJPEG is disabled :-)
</pre>
      </blockquote>
      <pre wrap="">
Sure, this fixes the issue (and we should commit it to help with the infinite 
recursion issue), but what's your opinion on making our mutexes recursive?</pre>
    </blockquote>
    I think, a quite good discussion about recursive and non recursive
    mutes was here:
    <a class="moz-txt-link-freetext" href="http://stackoverflow.com/questions/187761/recursive-lock-mutex-vs-non-recursive-lock-mutex">http://stackoverflow.com/questions/187761/recursive-lock-mutex-vs-non-recursive-lock-mutex</a>.
    This convinced me to vote for a usage of recursive mutex in poppler:<br>
    1. xpdf and therfore poppler wasn't really designed for the usage of
    threads. We have it now working, but if we cherish on the usage of
    non recursive mutex we will always run into dead lock problems with
    futur patches again, so testing patches will become more expensive.<br>
    2. Since poppler nowhere uses emergency exits with "throw", we don't
    have the problem that we have to keep track that a thread releases a
    mutex so often it aquires it.<br>
    3. The only (small) problem I see is the sentence "Not all systems
    supporting pthreads also support recursive mutexes, but if they want
    to be <a
href="http://www.opengroup.org/onlinepubs/009695399/functions/pthread_mutexattr_gettype.html">POSIX
      conform, they have to</a>."<br>
    <br>
    Cheers,<br>
    Thomas<br>
    <blockquote cite="mid:9416343.zB127GlF5Q@xps" type="cite">
      <pre wrap="">

It should help us making the code simpler since we could just drop all the 
DoNotLockMutex cases, no?

Cheers,
  Albert

</pre>
      <blockquote type="cite">
        <pre wrap="">
Cheers,
Thomas

</pre>
        <blockquote type="cite">
          <pre wrap="">Cheers,
Thomas

</pre>
          <blockquote type="cite">
            <blockquote type="cite">
              <pre wrap="">Wouldn't a real recursive mutex be enough?
</pre>
            </blockquote>
            <pre wrap="">
No idea: I used in the thread safe patch just the pthread_lock which
was defined. I'm not knowing what happens when we change it globally
i.e. if it affects other usages of the GooMutex. Just feel free to
change it, You should have the better overview of usages than I have.

Cheers,
Thomas

</pre>
            <blockquote type="cite">
              <pre wrap="">I'm asking because i have a document here (that sadly i can't share)
that is deadlocking
itself there because we are not passing the recursion integer
everywhere (we lose it in DCTStream::DCTStream)
as shown in this backtrace

#0  __lll_lock_wait () at
.../nptl/sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:135
#1  0x00007ffff599217c in _L_lock_982 () from
/lib/x86_64-linux-gnu/libpthread.so.0
#2  0x00007ffff5991fcb in __GI___pthread_mutex_lock (mutex=0x71dc10)
at pthread_mutex_lock.c:64
#3  0x00007ffff5f02f73 in MutexLocker::MutexLocker
(this=0x7fffffffcfb0, mutexA=0x71dc10, modeA=DoLockMutex) at
/home/tsdgeos/devel/poppler/goo/GooMutex.h:72
#4  0x00007ffff5fac8e2 in XRef::fetch (this=0x71db50, num=5, gen=0,
obj=0x7fffffffd0b0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/XRef.cc:1137
#5  0x00007ffff5f8a013 in Object::fetch (this=0x71fe08,
xref=0x71db50, obj=0x7fffffffd0b0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Object.cc:122
#6  0x00007ffff5f19de4 in Dict::lookup (this=0x71fb60,
key=0x7ffff606fbba "Height", obj=0x7fffffffd0b0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Dict.cc:256
#7  0x00007ffff5f032e9 in Object::dictLookup (this=0x7fffffffd5a0,
key=0x7ffff606fbba "Height", obj=0x7fffffffd0b0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Object.h:315
#8  0x00007ffff601f8a3 in DCTStream::DCTStream (this=0x7832b0,
strA=0x720780, colorXformA=-1, dict=0x7fffffffd5a0) at
/home/tsdgeos/devel/poppler/poppler/DCTStream.cc:72
#9  0x00007ffff5f9bf6d in Stream::makeFilter (this=0x720780,
name=0x71fef0 "DCTDecode", str=0x720780, params=0x7fffffffd1e0,
recursion=1, dict=0x7fffffffd5a0) at
/home/tsdgeos/devel/poppler/poppler/Stream.cc:311
#10 0x00007ffff5f9b71a in Stream::addFilters (this=0x720780,
dict=0x7fffffffd5a0, recursion=1) at
/home/tsdgeos/devel/poppler/poppler/Stream.cc:184
#11 0x00007ffff5f9204b in Parser::makeStream (this=0x71fcd0,
dict=0x7fffffffd5a0, fileKey=0x0, encAlgorithm=cryptRC4,
keyLength=1146103040, objNum=3, objGen=0, recursion=1, strict=false)
at /home/tsdgeos/devel/poppler/poppler/Parser.cc:280
#12 0x00007ffff5f918e6 in Parser::getObj (this=0x71fcd0,
obj=0x7fffffffd5a0, simpleOnly=false, fileKey=0x0,
encAlgorithm=cryptRC4, keyLength=1146103040, objNum=3, objGen=0,
recursion=0, strict=false) at
/home/tsdgeos/devel/poppler/poppler/Parser.cc:131
#13 0x00007ffff5facdd3 in XRef::fetch (this=0x71db50, num=3, gen=0,
obj=0x7fffffffd5a0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/XRef.cc:1197
#14 0x00007ffff5f8a013 in Object::fetch (this=0x71e128,
xref=0x71db50, obj=0x7fffffffd5a0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Object.cc:122
#15 0x00007ffff5f19de4 in Dict::lookup (this=0x71e660, key=0x7206e0
"Im1", obj=0x7fffffffd5a0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Dict.cc:256
#16 0x00007ffff5f032e9 in Object::dictLookup (this=0x71e598,
key=0x7206e0 "Im1", obj=0x7fffffffd5a0, recursion=0) at
/home/tsdgeos/devel/poppler/poppler/Object.h:315
#17 0x00007ffff5f2a647 in GfxResources::lookupXObject
(this=0x71e590, name=0x7206e0 "Im1", obj=0x7fffffffd5a0) at
/home/tsdgeos/devel/poppler/poppler/Gfx.cc:411
#18 0x00007ffff5f3f41e in Gfx::opXObject (this=0x71e440,
args=0x7fffffffd720, numArgs=1) at
/home/tsdgeos/devel/poppler/poppler/Gfx.cc:4114
#19 0x00007ffff5f2bfd6 in Gfx::execOp (this=0x71e440,
cmd=0x7fffffffd6e0, args=0x7fffffffd720, numArgs=1) at
/home/tsdgeos/devel/poppler/poppler/Gfx.cc:858
#20 0x00007ffff5f2b82b in Gfx::go (this=0x71e440, topLevel=true) at
/home/tsdgeos/devel/poppler/poppler/Gfx.cc:717


Cheers,

   Albert

_______________________________________________
poppler mailing list
<a class="moz-txt-link-abbreviated" href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/poppler">http://lists.freedesktop.org/mailman/listinfo/poppler</a>

..
</pre>
            </blockquote>
            <pre wrap="">
_______________________________________________
poppler mailing list
<a class="moz-txt-link-abbreviated" href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/poppler">http://lists.freedesktop.org/mailman/listinfo/poppler</a>

..
</pre>
          </blockquote>
          <pre wrap="">
_______________________________________________
poppler mailing list
<a class="moz-txt-link-abbreviated" href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/poppler">http://lists.freedesktop.org/mailman/listinfo/poppler</a>

..
</pre>
        </blockquote>
      </blockquote>
      <pre wrap="">_______________________________________________
poppler mailing list
<a class="moz-txt-link-abbreviated" href="mailto:poppler@lists.freedesktop.org">poppler@lists.freedesktop.org</a>
<a class="moz-txt-link-freetext" href="http://lists.freedesktop.org/mailman/listinfo/poppler">http://lists.freedesktop.org/mailman/listinfo/poppler</a>

..

</pre>
    </blockquote>
    <br>
  </body>
</html>