New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUPS cached data (string pool, languages, transcoding data) should be global #1530

Closed
michaelrsweet opened this Issue Mar 31, 2006 · 10 comments

Comments

Projects
None yet
1 participant
@michaelrsweet
Collaborator

michaelrsweet commented Mar 31, 2006

Version: 1.2rc1
CUPS.org User: twaugh.redhat

Applications using libgnomeprint, such as gedit, do not work with CUPS 1.2rc1 at all, but work fine with CUPS 1.1.23.

It seems to be some sort of thread-safety issue. The CUPS_GET_PRINTERS request works fine (once the _cupsStrFree() issue I reported earlier is patched), but then the CUPS_GET_PRINTER_ATTRIBUTES requests, which are issued several at once from different threads, become garbled.

The result is that no CUPS queues are shown in the dialog.

Steps to reproduce:

  1. Start gedit
  2. Control-P
@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 1, 2006

CUPS.org User: mike

Since previous versions of CUPS were not thread-safe, I don't think this is the issue, however I can reproduce the problem and will look into it...

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: twaugh.redhat

FWIW, putting 'return strdup (s);' at the very beginning of _cupsStrAlloc() makes the problem go away..

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

What version of GNOME are you running?

I did my testing with the version included with FC4 (2.10.x); I don't see any problems with the 2.14 code, but it is possible they are reusing the string pointer without actually g_strdup'ing it, or perhaps g_strdup() is trying to optimize things by not actually strdup'ing an allocated buffer?

If I set the initial reference count to 2, then the problem also goes away...

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

OK, I'm running Valgrind on gedit from GNOME 2.10.x and getting a LOT of errors. When I instrument _cupsStrFlush(), I'm getting it called multiple times for the same thread (or at least the same globals pointer...)

Also, if I comment out the _cupsStrFlush() code, then everything works (but still lots of duplicate frees in the GNOME code...)

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

OK, my current working theory is that GNOME is accessing request data created by another thread; when the thread goes away, CUPS deletes the per-thread data and we see the problem.

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

I'm renaming this STR to reflect the actual problem.

I'm not sure where to proceed from here; the string pool stuff is a huge performance and memory usage win so we are definitely not going to go back to strdup and free.

We might be able to update the implementation to use a single, global (all threads) string pool. We'll need to add a mutex for the string pool, but it should work...

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

[changed STR summary again...]

OK, looks like using a global string pool with a mutex protecting it does the trick. I'll do the same for the cups_lang_t and transcoding caches and commit later today.

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

OK, I've commited a fix for this in r5366. Basically, all of the string pool, languages, and transcoding data is now shared among threads.

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: twaugh.redhat

Great, works fine here! Thanks for fixing this quickly.

Yes, the string pool is a great idea, and I wouldn't suggest abandoning it.

Another performance-only idea is to store large lists (printers, for example) in hash tables.

@michaelrsweet

This comment has been minimized.

Collaborator

michaelrsweet commented Apr 2, 2006

CUPS.org User: mike

Fixed in Subversion repository.

@michaelrsweet michaelrsweet added this to the Stable milestone Mar 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment