| | <<<Back 1 day (to 2015/10/22) | 20151023 |
| chrisl | kens: could you have a peruse of http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=3ff82bf9 and see if you're okay with it? | 09:10.02 |
| kens | One second | 09:12.12 |
| | Ah I thnk this may fix a coverity warning I've been thinking about for a while..... | 09:12.53 |
| | Yes that looks entirely reasonable ot me | 09:13.14 |
| chrisl | I'd be surprised (and impressed) if static analysis noticed that.... | 09:13.36 |
| | Thanks | 09:13.40 |
| kens | NP | 09:13.51 |
| chrisl | Clearly, whoever wrote the t1 charstring code mistakenly thought they could believe what the spec said! | 09:14.41 |
| kens | Yeah it slooks like its taken straight from the spec :-( | 09:15.03 |
| chrisl | Well, IIRC, the spec says that the interpreter can be considered "privaleged" code, and thus apply absolute minimal error checking - clearly not the case! | 09:15.53 |
| kens | The spec does say that, but clearly its never been the case for Adobe interpreters, or these fonts would have failed long ago | 09:16.22 |
| chrisl | This is for a fuzzing bug, so the font data is garbage | 09:17.13 |
| kens | Oh well fair enough | 09:17.58 |
| | Looks like you need to modify the API.htm document :-) | 09:18.12 |
| chrisl | Ugh :-( | 09:18.22 |
| kens | Well the bug 696301 is complaining about a Gentoo bug to do with building stuff that depends on GS | 09:18.53 |
| | Because it now needs gserrors.h | 09:19.00 |
| | And they are using SPI.htm which says you can build with ierrors.h to call it a regression | 09:19.19 |
| | Seems to me the simple solution is to change the documentation | 09:19.29 |
| chrisl | Well, no, because gserrors.h genuinely is missing from the installed files | 09:19.52 |
| kens | Oh! | 09:19.58 |
| chrisl | Just using ierrors.h will still work, *IF* gserrors.h is also there | 09:20.33 |
| | I have a commit here that fixes it, and I'll push it in a few minutes | 09:21.03 |
| kens | Well, its nothing to do with me :-) | 09:21.05 |
| | Ah I see you fixed that outrageous intentional seg fault chrisl, I kept forgetting about it :-) | 09:46.57 |
| chrisl | Yeh, as did I - until yesterday! | 09:47.21 |
| kens | Glad to see it sorted | 09:48.02 |
| chrisl | I don't see what the problem was, TBH. After all, it was done by Igor, and he was a *very* productive developer..... <sigh> | 09:49.29 |
| Robin_Watts | In case anyone is interested, ebuyer are doing a 240GB SSD for 49.99 today. | 11:49.17 |
| kens | That's a good price | 11:49.40 |
| yitzchok | ah are you guys mostly in the UK? | 11:59.38 |
| chrisl | yitzchok: at the moment, yes - the US folks will start to appear as the afternoon passes | 12:01.15 |
| yitzchok | well that makes sense | 12:01.37 |
| chrisl | And one of us in Sweden | 12:01.40 |
| yitzchok | just wondering after the ebuyer comment | 12:02.26 |
| Robin_Watts | yitzchok: Where are you based? | 12:02.42 |
| yitzchok | Israel, but UK born and bred | 12:03.32 |
| | going to try debugging the Brother/Linux printing process now | 12:04.51 |
| chrisl | yitzchok: I haven't been paying attention to your printing issue (so, sorry if this is a repetition): are you sending Postscript to the printer? | 12:06.29 |
| yitzchok | I'm not entirely sure :) | 12:06.59 |
| | I'm printing from Linux desktop to Linux server using CUPS | 12:07.22 |
| chrisl | Ah..... does't the CUPS configuring dialogue give a hint? | 12:07.37 |
| yitzchok | Then the Brother CUPS filters take it | 12:07.42 |
| | There is a variable PCL= in the filter script but I don't remember now what it affects | 12:08.41 |
| | CUPS options don't refer to PS AFAIR | 12:09.12 |
| chrisl | For my HP, in the printer properties "Make and Model" firled it says: "HP LJ 300-400 color M351-M451 Postscript (recommended)" | 12:09.39 |
| yitzchok | Inside the Brother scripts is a call to GS which outputs to bit device | 12:10.11 |
| | Driver in CUPS is called "Brother HL2250DN for CUPS" | 12:10.23 |
| kens | not sending PS then | 12:10.34 |
| chrisl | Ah, so probably not Postscript then, if it's converting to "bit" | 12:10.37 |
| | Okay, looking at the specs, it is probably wrapping a "bit" output in PCL6/XL | 12:11.50 |
| | Grabbing lunch.... bbiab | 12:12.23 |
| yitzchok | I'm going to go through it again step by step but the problem I saw last time was that something (PS I think) was passed to GS and nothing came out | 12:12.48 |
| kens | Probably should have raied an error, if you can capture the PostScript we cna look at that | 12:13.39 |
| yitzchok | The PS passed to GS? By all means | 12:14.05 |
| kens | Yes that's the thing. It may have been *created* by GS as well, or it could just be a PDF, CUPS uses PDF now internally I believe | 12:14.33 |
| yitzchok | I will trace it through | 12:14.43 |
| kens | The main thng is to capture what gets sent to GS :) | 12:15.03 |
| verdule | pardon here, i want to break thru to get your attention. | 12:32.56 |
| | here's my pastebin reference for any of you interested: http://pastebin.com/TW5QR028 | 12:33.12 |
| kens | What's ths about ? | 12:33.30 |
| verdule | Sorry to pester you all but I want to be honest. I really don't know how to contact you or where there's a public email address. Why did you kick me off the mailing list? I didn't know how to reach you! | 12:34.31 |
| kens | I have no idea what you are referring to | 12:35.01 |
| | What mailing list ? | 12:35.08 |
| verdule | The question is about a part of the ghostscript package, merely the ps2pdf command. Did you follow the pastebin link? | 12:35.59 |
| kens | No, I wasn't going to until I discovered what you were talking about | 12:36.17 |
| | No point if it s not relevant to me | 12:36.26 |
| chrisl | verdule: The font that gets used is set by the *input* to Ghostscript, not by Ghostscript itself | 12:36.44 |
| kens | Now you're talking about a mailing list, which list are you referring to ? | 12:36.48 |
| verdule | The mailing list had entities such as gs-devel and the like. The relevant page said some of these were public email address and I thought so. Then I got kicked out... | 12:37.05 |
| kens | I'm not aware we've done that. | 12:37.30 |
| verdule | anyway man pages don't really have fonts of their own do they? And I was piping out a man page with -t option into ps2pdf. | 12:37.44 |
| kens | Possibly you had a 'too many bounces' message and were unsubscribed | 12:37.45 |
| | verdule, it doesn't matter whether man pages have fonts or not. | 12:38.00 |
| | If you print them to PostScript or PDF, then that must reference a fotn to draw the text | 12:38.19 |
| | Which font gets used (or requested) isn't up to us | 12:38.36 |
| chrisl | verdule: the "-t" option tells man to output Postscript, and the Postscript from man contains a request for "Times-Roman" - Ghostscript is doing what it's told | 12:38.39 |
| verdule | The man page for ps2pdf told me any option available beside those described in its own man page, are the same as in gs proper. | 12:38.42 |
| kens | Yes, but that doesn't seem to be relevant here. | 12:38.58 |
| verdule | Of course ghostscript was doing what was being told. | 12:39.07 |
| kens | You are giving Ghostscript a PostScript program whch says 'use TImes-Roman' and it does..... | 12:39.19 |
| chrisl | Erm, well, what's your question then? | 12:39.23 |
| verdule | I want to know what command line option I need to change the reference font to a monospace such as Courier New. How do I do it? | 12:39.44 |
| kens | THere is no command line option to Ghostscript (or ps2pdf) that says 'when I ask for Times-Roman use Courier instead' | 12:39.54 |
| chrisl | You change the input Postscript | 12:39.55 |
| kens | Remember PostScript is a programming language | 12:40.08 |
| verdule | How about also resizing the body text? | 12:40.11 |
| chrisl | You change the Postscript | 12:40.20 |
| kens | Same thing, change the PostScript program | 12:40.23 |
| verdule | Of course PostScript is a language; I fed the whole thing thru ps2pdf to make a PDF from a man page. The font that came out was Times-New-Roman. I want monospace such as Courier New instead... | 12:41.16 |
| | What PostScript program are you talking about? ps2pdf? | 12:41.30 |
| kens | No | 12:41.39 |
| chrisl | The thing you feed into ps2pdf is a Postscript program | 12:41.46 |
| kens | The PostScript you produce from the man page | 12:41.47 |
| verdule | Okay. But I can't make another PostScript with the font I want in it. | 12:42.45 |
| kens | I don't really see how we can help you with that | 12:42.58 |
| verdule | Perhaps I should try man -t grep > choose_font.ps first. | 12:43.38 |
| chrisl | The font selection is *in* the Postscript that man produces | 12:43.56 |
| verdule | then next something like ps2pdf -sPAPERSIZE=a4 - man_grep.pdf or whatever. | 12:44.23 |
| kens | I have no idea why any of that will help | 12:44.35 |
| chrisl | And by the looks of it, it does the selections lots and lots of times | 12:44.39 |
| verdule | Shall I make a PostScript file first so I can edit that as a text file to manipulate the font selection? | 12:45.05 |
| | And where are the fonts I can choose from? Are they coming from some standard list of fonts? | 12:45.32 |
| kens | Simply changing the font name *might* work, or it might simply mess up the formatting altogether | 12:45.36 |
| | verdule I'm sorry to say that if you want to alter a PostScript program meaningfully, you will need to learn some PostScript | 12:46.08 |
| chrisl | And you have to change references to all the font family (bold, italic etc) - and as I said, the Postscript selects the fonts many, many times | 12:46.30 |
| kens | The PostScript Language Reference Manual is avilable on the web as a PDF file | 12:46.38 |
| verdule | Remember I already have the PDF with the serify Times-New-Roman font. I'm not really all that interested in PostScript. | 12:46.42 |
| kens | Chaninbg a PDF file is even more challenging | 12:46.57 |
| | na dyou are producing PostScript anyway | 12:47.08 |
| verdule | Maybe I should just leave my PDF as-is. And not bother with making it look more sensible. | 12:47.08 |
| kens | That would be the sensible option I would say | 12:47.23 |
| | Or complain to the maintainers of man that they should use Courier instead of Times-Roman | 12:47.49 |
| chrisl | Perhaps you should talk to the authors of "man" and see if there is a configuration option for it | 12:47.52 |
| verdule | Look I wanted to change the font in the PostScript before it becomes a PDF. | 12:47.58 |
| kens | And I just told you, if you want to d that you'll have to learn some PostScript. As well as the fact that doing so may simply mess up the formatting totally | 12:48.36 |
| | PostScript is a programming language, you can't change a program unless you have some idea what you are doing | 12:49.04 |
| verdule | My lookup of the man page for man says that with the -t option the output is either troff or groff. And that's a different story altogether isn't it? | 12:49.20 |
| kens | And it may be easier to create a new program than understand and alter an existing one | 12:49.21 |
| | Yes, troff and groff are not PostScript | 12:49.37 |
| | Though I thnk they can *produce* PostScript | 12:50.24 |
| verdule | The .ps file is actually a groff output. | 12:50.26 |
| kens | Right, so there you go, look at groff, maybe you can do somethign with that | 12:50.44 |
| verdule | Look this irc talk is very fast paced. | 12:51.17 |
| kens | Ths isn't us being unhelpful, there genuinely is little or nothing we cna do to help, you need ot 'fix' your problem at an earlier stage | 12:51.30 |
| verdule | I see many references in my man-to-pdf preparation file many references to the same font: Times. | 12:52.45 |
| | I'll do a search-and-replace automation to change this to Courier. What do you think? | 12:54.03 |
| kens | I gebuinely have no idea | 12:54.23 |
| verdule | Well thank you for trying to help me out. | 12:54.41 |
| kens | we dop try to help | 12:55.00 |
| chrisl | Changing "Times" to "Courier" will result in trying to use "Courier-Roman", so you'll need to be a bit more careful | 12:55.18 |
| kens | No idea why you were removed from a mailing list unless you got the too many bounces message | 12:55.19 |
| verdule | Okay. | 12:56.11 |
| chrisl | You'll have to search and replace individually for Times-Roman, Times-Bold, Times-Italic (maybe Times-BoldItalic?) - it *might* work | 12:57.03 |
| kens | depends on formatting I guess | 12:57.23 |
| verdule | Maybe I can try out man -H grep > man_grep.html instead. | 12:57.23 |
| | No bold-italic combos in it yet :) | 12:58.13 |
| chrisl | No, I didn't see any, but if you're scripting it, might as well cover the bases | 12:58.39 |
| | kens: Another commit for your approval (or otherwise): http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=commitdiff;h=fb1154ad | 12:59.32 |
| verdule | I think my alteration should do for now. | 12:59.37 |
| kens | Ah I thnk I did a little of ths before chrisl | 12:59.59 |
| | Yeah that looks entirely sensible | 13:00.28 |
| verdule | Font change worked, but lines are getting cut off past the page. | 13:00.35 |
| kens | verdule, yes that was what IO meant when I said the fomratting might break with a simple change | 13:00.54 |
| chrisl | kens: Ta | 13:00.57 |
| verdule | What does that git script do? | 13:03.54 |
| kens | git script ? | 13:04.05 |
| verdule | Which you referenced a few minutes ago. | 13:04.09 |
| kens | I don't remember referencing that, could be a typo | 13:04.27 |
| verdule | The formatting's stuffed; I can't get it to reflow. I'll go the HTML route instead of the PostScript route. Won't /that/ be easier for me? | 13:05.30 |
| kens | I don't know, it depends what you're trying to achieve I guess | 13:05.47 |
| | If you really need a PDF, then htmltopdf solutions don't really work all that well | 13:06.20 |
| verdule | Everything's a mess. | 13:07.03 |
| kens | Seems ot me that's because you are trying to use tools to do somethign they weren't intended for | 13:08.13 |
| verdule | Very well. I think I ought to leave you. Thanks for your best intentions of helping me out with making a nicer printout man page re-formatting. | 13:11.10 |
| chrisl | Ah well, my final suggestion not required, then...... | 13:12.35 |
| yitzchok | Well, I just finished working through a test and got the PS file I thought gs wouldn't process and it did. That said, I got that file by printing (to a paused queue), and when I printed the same file with the printer not paused it didn't print - but if ran the gs output through the Brother program (the rest of the CUPS filter) and then threw that output at the printer, it printed. Odd. | 13:33.56 |
| | Now I'm not sure what's different. I'll have to go through this again, but I can't now. | 13:34.01 |
| | Thanks, guys, for being willing to help. When I have something reproducible, I'll come back. | 13:34.28 |
| kens | Np | 13:35.23 |
| yitzchok | (BTW my testing was with the binary 9.18 download from the ghostscript site - and when I found that it handled the file OK I ran it through 9.05 from my distro and it came out OK too....) | 13:35.32 |
| chrisl | yitzchok: you may have seen this already, but there may be some useful hints here: https://wiki.ubuntu.com/DebuggingPrintingProblems | 13:35.55 |
| yitzchok | yep I've been using that to help me grab the files | 13:36.12 |
| | Thanks | 13:36.24 |
| | Must be off now. Have a good weekend | 13:36.52 |
| kens | bye bye | 13:36.58 |
| chrisl | Interesting: my HP printer has a PS backchannel..... | 13:45.02 |
| kens | and it works ? | 13:51.15 |
| chrisl | Yes, if I use netcat to send PS to the printer, netcat gets the backchannel back and prints it on the terminal | 13:52.14 |
| kens | Hmm | 13:52.26 |
| | well, might be useul one day | 13:54.07 |
| chrisl | Well, maybe. "Unfortunately" it's not a printer that has any trouble with our PS output, so it doesn't help with that | 13:55.36 |
| kens | "yet" | 13:56.03 |
| chrisl | I've printed a decent number of files to it.... | 13:56.32 |
| henrys | I was looking yesterday at copy-paste errors in coverity, looks like some good catches there, have you guys seen those? | 14:11.18 |
| kens | I only look at the reports by file, so I can pick out the ones I own | 14:12.48 |
| henrys | ttinterp.c have a couple real ones and gxclimag.c. | 14:17.58 |
| | I was just surprised they could detect that sort of thing without a huge number of false-positives. | 14:19.36 |
| chrisl | I'm guessing the one in ttinterp.c predates our importing of the code - and I doubt we use that any more, anyway | 14:23.50 |
| kens | Might do for pdfwrite, not certain | 14:24.21 |
| chrisl | I thought it only used the internal stuff for metrics? | 14:24.59 |
| kens | I was thinkning of the subsetting, but I'm not at all sure | 14:25.12 |
| chrisl | Why would it run the bytecode interpreter for subsetting? | 14:25.40 |
| kens | compound glyphs | 14:25.51 |
| chrisl | That doesn't make sense.... | 14:26.14 |
| kens | I haven't looked at the code to see whaerre it is | 14:26.27 |
| chrisl | This is in the TTF hinting code - the bytecode interpreter | 14:26.44 |
| kens | Then I'm sure we don';t use it | 14:28.16 |
| mvrhel_laptop | Robin_Watts: are you around? | 14:30.02 |
| kens | mvrhel_laptop : did you have a chance to try Windows 10 ? | 14:30.04 |
| mvrhel_laptop | kens: just getting ready to do that | 14:30.13 |
| | I made a new installer | 14:30.16 |
| kens | ah OK thanks | 14:30.19 |
| mvrhel_laptop | and I wanted to get it signed | 14:30.20 |
| Robin_Watts | mvrhel_laptop: I am here. | 14:30.25 |
| kens | will be interested to hear if you see any problmes | 14:30.28 |
| mvrhel_laptop | oh hi Robin_Watts | 14:30.34 |
| | I wanted to sign my installer. But the key file I think has a password? | 14:31.01 |
| | I got the pfx etc files from you but I had never done the signing | 14:31.29 |
| | At one point you had the tools to do the signing on casper but I don't see them there now | 14:32.05 |
| | Last may you said they were in ~robin/Certificates/WDKSignTools.zip | 14:32.39 |
| Robin_Watts | Sorry, on hold on phone, so I periodically get distracted as I yell at incompetent muppets. | 14:33.57 |
| mvrhel_laptop | Robin_Watts: no problem when you are done is fine | 14:34.19 |
| kens | you still fighting about washing machine Robin_Watts ? | 14:35.07 |
| Robin_Watts | You want to look at /home/git-private/Certificate | 14:35.20 |
| | You want to look at /home/git-private/Certificates/README :) | 14:35.45 |
| | kens: Yes. 3 hours yesterday to get them to agree to replace it. | 14:36.38 |
| kens | :-( | 14:36.45 |
| Robin_Watts | Then someone was supposed to call us back to arrange a date. They didn't, so Helen rang, and they've "lost" the magic uplift number. | 14:37.08 |
| | Which fortunately I have written on a piece of paper here. | 14:37.21 |
| kens | :-) | 14:37.32 |
| Robin_Watts | And it's all very confusing for their tiny little minds, cos it was originally delivered to Tims workshop, but of course is not there any more... | 14:37.54 |
| mvrhel_laptop | Robin_Watts: ah ok thanks! | 14:38.03 |
| kens | Surely they can understand delivery top a location and movement, must happen all the time | 14:38.22 |
| | taking lessons from AT&T.... | 14:38.47 |
| henrys | chrisl: the ttinterp stuff does show up in coverage so somebody is using it? | 14:46.03 |
| | chrisl: I was curious myself why that was | 14:46.27 |
| | mvrhel_laptop: looks like you have a "real" coverity problem too, it suggests gxclimage.c:688 should be cmyk_rend_cond vs rgb_rend_cond | 14:53.40 |
| chrisl | henrys: I'll look at it, I guess..... | 14:54.02 |
| mvrhel_laptop | henrys: oh ok | 14:54.48 |
| | kens: windows 10 works fine | 14:54.53 |
| kens | mvrhel_laptop : Thanks for that! | 14:55.04 |
| henrys | chrisl: according the to the coverage it's not just pdfwrite. puzzling | 14:55.18 |
| kens | I was 'reasonably sure' I'd tested it before but its nce to have confirmation | 14:55.21 |
| | stick a deliberate seg fault, run a cluster test, see if anything falls over | 14:56.02 |
| chrisl | Doing that now | 14:56.16 |
| kens | :-) | 14:56.27 |
| chrisl | But apparently my typing is starting to suffer :-( | 14:58.36 |
| mvrhel_laptop | chrisl: I want to give you a new installer to put up for gsview | 14:58.58 |
| | let me stick it on casper | 14:59.06 |
| henrys | oops it does look look only high level devices use that code. | 14:59.20 |
| | so kens problem ;-) | 14:59.51 |
| chrisl | Still, I don't think even high level devices should be using it to render glyphs | 14:59.55 |
| kens | I don't thnk they should be suing it at all, I'd be interested to know why (if they genuinely are using the bytecode interpreter) | 15:00.26 |
| henrys | if you go to the coverage window and hover over stuff it gives you a list of files then you can make a command and use a breakpoint. Just my 2 cents | 15:00.48 |
| kens | is inclined to ignore it until someone can provide a failing example | 15:01.42 |
| mvrhel_laptop | chrisl: ok it is in ~mvrhel/gsview/ | 15:02.01 |
| chrisl | mvrhel_laptop: Okay, I'll put it where it needs to be | 15:02.19 |
| mvrhel_laptop | that includes some forms support, gproof, as well as a pile of other fixes including zooming to 64x | 15:02.43 |
| henrys | kens, chrisl I wonder if the coverage is bonked or something this is showing hundreds of pdf and postscript files using this code to render glyphs for pdfwrite. | 15:04.25 |
| kens | well that sounds suspicous to me | 15:04.56 |
| mvrhel_laptop | thanks chrisl | 15:05.09 |
| | ok I will look at my coverity issues | 15:05.19 |
| | I am sure there are a pile of them | 15:05.28 |
| | first need to eat breakfast... | 15:05.44 |
| henrys | mvrhel_laptop: sorry I got sick of the dreaded language switching yesterday and started looking at that. I should have played basketball or something instead ;-) | 15:06.13 |
| mvrhel_laptop | hmm what should my reply to 696276 be? | 15:39.44 |
| | suggest they try gsview 6.0, send them to Russell, ask for a file? | 15:40.31 |
| kens | I'd tell them to use the new code. We (and I believe russel) won't be updating the old code | 15:40.59 |
| mvrhel_laptop | right | 15:41.55 |
| henrys | kens: I think I'm up to date on your pjl changes LGTM. | 16:19.30 |
| kens | OK thanks henry | 16:19.41 |
| | Still working on distillerparams but they will be more or less exactly the same just more omcplicated | 16:20.02 |
| rayjj | Robin_Watts: chrisl (or any others): I mentioned after your day yesterday that I'd like a quick review on my patch on my repo for bug 696254 (the intermittent multi-threading crash) | 16:27.47 |
| Robin_Watts | rayjj: You did. and I looked over it and burbled all over the logs. | 16:28.07 |
| rayjj | Robin_Watts: Oh, I didn't expect a comment in yesterday's logs. Sorry | 16:28.32 |
| henrys | kens, chrisl: did you guys review or suggest shelly's change to pdf_draw.ps? anyway one of you should look at it: http://git.ghostscript.com/?p=user/shailesh/ghostpdl.git;a=commitdiff;h=c9b540e6e1ea1b55ff3df8da51e99cc874306d24. I'll review the rest | 16:29.26 |
| chrisl | henrys: I pointed out that we had to parse parts of the stream in PS, but I hadn't realised he'd got the stage of doing a patch (he's been busy with other things) | 16:30.43 |
| | The Postscript is fine, I'm assuming that the number decoding is correct.... | 16:32.48 |
| henrys | chrisl: the business of updating a single function from openjpeg 2.2 is bothersome | 16:35.05 |
| chrisl | henrys: well, I'm not sure what's involved in updating everything to 2.2, and whether we want to devote the effort to that right now...... | 16:36.44 |
| | The next release *should* have the memory manager changes in it, so OPJ can use our memory management | 16:37.17 |
| henrys | chrisl: I was thinking we don't need it for the luratech fix for the customer, why not just wait, but I don't feel strongly about it. | 16:38.38 |
| | chrisl: I'll talk to him about it. | 16:41.04 |
| chrisl | henrys: I don't feel terribly strongly either. Except maybe it's a stronger position to detect regressions when we do update to the next release | 16:42.00 |
| | henrys: actually maybe 2.2 is going to be the next OPJ release - might be best to clarify that before deciding | 16:45.31 |
| rayjj | Robin_Watts: thanks for the review. | 16:46.41 |
| Robin_Watts | rayjj: Are you going to answer my questions ? | 16:46.57 |
| | As far as I could tell, the file reading was done using ReadFile, which gives a specific file offset to go to. | 16:47.28 |
| | What was accessing the file that was relying on the file pos ? | 16:47.51 |
| chrisl | I don't understand how the file size can change during the reading phase | 16:48.23 |
| Robin_Watts | My understanding of the patch is that "we no longer seek to the end to ftell the length of the file, cos that's not atomic" | 16:48.56 |
| rayjj | Robin_Watts: I added debug code that does the seek to SEEK_END before the call to cl_cache_read_init, then compares icf->filesize to the return from gp_ftell64(icf->f) and break on the statement, and I get breakpoints at different (indeterminiate) number of threads running | 16:49.37 |
| Robin_Watts | Which is fair enough, but begs the question "if nothing else is touching the file position, what does it matter if it's not atomic" | 16:50.01 |
| chrisl | Oh, then I misunderstood how Paul's code worked..... | 16:50.17 |
| rayjj | Robin_Watts: so something is able to move the FILE * position. I didn't think ReadFile did, but maybe it does ? | 16:50.23 |
| Robin_Watts | rayjj: That sounds an awful lot like "I don't know, but this seems to fix it" | 16:50.54 |
| rayjj | Well, ReadFile is in the kernel and can't be traced, | 16:51.16 |
| | Robin_Watts: I admit to not knowing who is moving the file position | 16:51.57 |
| Robin_Watts | rayjj: You can test whether ReadFile moves the file pointer dead easily. In a single threaded run fseek somewhere odd, then ReadFile then ftell to see if the file pointer was updated. | 16:52.10 |
| | rayjj: Then I submit that you can't be sure this is a proper fix. | 16:52.25 |
| rayjj | Robin_Watts: true, I'll try that | 16:52.26 |
| Robin_Watts | It might well reduce the frequency of crashes, but... | 16:52.48 |
| rayjj | Robin_Watts: oh, I can trace into the CRTL _ftelli64 and _ftelli64_nolock. ftelli64 acquires a lock on the stream, but then ftelli64_nolock does: | 17:01.27 |
| | fd = _fileno(stream); | 17:01.28 |
| | filepos = _lseeki64(fd, 0i64, SEEK_CUR); | 17:01.30 |
| | and ReadFile uses the "hande" from _get_os_fhandle(fileno(f)); | 17:01.31 |
| | so it is reasonable that ReadFile affects the position of the fd, but I'll test that | 17:01.33 |
| | Robin_Watts: OK, confirmed. ReadFile *does* affect the value returned by gp_ftell_64 | 17:07.43 |
| | Robin_Watts: it gets set to then position after the amount read | 17:08.26 |
| Robin_Watts | OK. So a ReadFile that interrupted the fseek(END) ftell() would explain the issue. | 17:08.52 |
| rayjj | I'll modify the comment to document that and post a revised commit for you to look at | 17:09.03 |
| Robin_Watts | And thus your fix seems entirely reasonable :) | 17:09.05 |
| | Ta. | 17:09.11 |
| | But... this does throw some interesting questions up. | 17:10.10 |
| rayjj | and that's why a large number of threads makes it more likely, since the cache_read_init happens when a thread starts | 17:10.11 |
| Robin_Watts | If every ReadFile takes a lock, it means that every thread blocks each other when reading from disc. | 17:10.46 |
| rayjj | "throw up" is an appropriate wording ;-) | 17:10.59 |
| | Robin_Watts: right, which is why the cache was needed | 17:11.25 |
| | and why lots of threads hits the point of diminishing returns sooner than the number of cores | 17:12.18 |
| | at least with -sBandListStorage=file | 17:12.35 |
| Robin_Watts | Kinda makes you wonder WTF the point of Overlapped IO is if it all gets serialised. | 17:12.52 |
| rayjj | well, presumably the OS also has cache for the file, so the lockout time is not that serious | 17:13.33 |
| Robin_Watts | rayjj: Unless... as you stepped in, did you see any tests on FILE_FLAG_OVERLAPPED ? | 17:14.00 |
| rayjj | Robin_Watts: not in the _file.c or _ftell64.c -- those always lock with: _lock_str(stream); | 17:15.38 |
| | and, I can't see the kernel level ReadFile code | 17:16.12 |
| | frankly, I'm surprised this hasn't come up sooner | 17:17.24 |
| mvrhel_laptop | hmm 696227 regression is odd. I wonder why my cluster test for for the original commit did not show this issue | 17:20.05 |
| Robin_Watts | rayjj: Oh, wait, so you *can't* see ReadFile taking a lock ? | 17:20.44 |
| rayjj | Robin_Watts: no, but it must be at least that smart, otherwise windows multiple thread access to a file would be totally broken -- even two threads doing just ReadFile from different posiitions | 17:22.25 |
| Robin_Watts | rayjj: We use the overlapped access to ReadFile. | 17:23.07 |
| rayjj | Robin_Watts: but the lock used is *not* the lock on the FILE * stream, but something that only relies on the os_fhandle | 17:23.10 |
| Robin_Watts | The whole point of that, I thought, was to ensure that reads did not block one another. | 17:23.35 |
| rayjj | Robin_Watts: if the fd's position is changing (from _lseeki64(fs, 0, SEEK_CUR) there must be a lock (at least around the setting of that position in the handle) | 17:26.20 |
| mvrhel_laptop | ok that is a bad regression | 17:26.22 |
| | something odd going on with the clist and my fix | 17:26.36 |
| Robin_Watts | rayjj: Where are you seeing lseeki64 called? | 17:26.50 |
| rayjj | Robin_Watts: see above, in the implementation of ftelli64_nolock in the CRTL | 17:27.19 |
| Robin_Watts | Yeah, I'm not following this. | 17:28.00 |
| | Windows has some low level kernel entry points like ReadFile. | 17:28.19 |
| | and it builds the CRT stuff (all the 'f' functions) on top of that. | 17:28.41 |
| | When you call ftell, then I expect it to take a lock. | 17:29.42 |
| | But I don't expect ReadFile to take a lock if the overlapped stuff is called. | 17:30.02 |
| rayjj | Robin_Watts: _ftelli64 takes a lock on the stream, then lseeki64 (which uses the fd) takes a lock on the handle: _lock_fh(fh); /* lock file handle */ | 17:30.44 |
| | but the ReadFile on the handle moves the position of the file handle | 17:31.33 |
| Robin_Watts | Right, but nothing you've said there proves that the ReadFile holds a lock during the time it is doing the read. | 17:32.16 |
| | (It may take one momentarily, but that's not the same as "take lock, wait for data to arrive, unlock" | 17:32.44 |
| rayjj | if we used int64_t filesize = _lseeki64(fd, 0, SEEK_END) we'd be fine | 17:32.54 |
| Robin_Watts | rayjj: That would be a less invasive change. But does this problem only happen on windows? | 17:33.30 |
| rayjj | Robin_Watts: no, ReadFile doesn't need to lock except while updating the fhandle position | 17:33.49 |
| Robin_Watts | I concur that ReadFile moves the file pointer. That does seem odd, but... hey ho. | 17:34.45 |
| rayjj | Robin_Watts: as far as other systems, only gp_unifs.c also returns true from gp_can_share_fdesc(f) | 17:37.35 |
| | Robin_Watts: and unix *only* uses operations on f, but still gp_fseek_64 ... gp_ftell_64 do not seem thread safe if something else can move the position of the FILE * (such as fread) | 17:39.07 |
| Robin_Watts | rayjj: Well, it's safe in the pread case, but not otherwise. | 17:40.03 |
| rayjj | Robin_Watts: I will put the code to check the file position of the seek/tell method against my patch's icf->filepos and see if I can get it to fail | 17:40.22 |
| Robin_Watts | So, for the unix, but no pread case, we should have a race problem, right? | 17:40.53 |
| rayjj | on linux, that is. With a large number of threads | 17:40.57 |
| Robin_Watts | Should we perhaps make gp_can_share_fdesc return 0 if no pread ? | 17:41.40 |
| rayjj | Robin_Watts: that's true assumng that unix ftell doesn't use the underlying fd == fileno(fd) as it does on Windows | 17:42.22 |
| Robin_Watts | rayjj: I don't follow that last statement. | 17:42.52 |
| | gp_fpread does: x=tell, seek, read, seek(x) | 17:43.35 |
| | in a non-atomic way. | 17:43.44 |
| | (in the absence of pread) | 17:44.00 |
| rayjj | Robin_Watts: right. Clearly would be non-threadsafe | 17:44.08 |
| Robin_Watts | If we have 2 threads trying to read data at the same time, it'll die. | 17:44.14 |
| | Hence I reckon your existing fix is enough for windows. | 17:44.23 |
| | but for linux, we should make gp_can_share_thingies only return 1 if pread is present. | 17:44.45 |
| rayjj | Robin_Watts: but if pread moves the position of the fd and ftello64 uses the position of the fd (rather than something maintained in the FILE * stream struct) it's the same problem as Windows | 17:45.37 |
| Robin_Watts | But your existing fix solves that for windows, right? | 17:46.15 |
| | And that's in non platform specific code, so surely that'll fix it for linux too? | 17:46.26 |
| rayjj | My fix avoids the issue by maintaining filesize in the gxclfie (any platform) | 17:46.55 |
| Robin_Watts | Right. | 17:47.00 |
| | So with your fix, the pread version is safe. | 17:47.13 |
| rayjj | Robin_Watts: it wouldn't take care of the race in the non pread case (as you point out) | 17:47.24 |
| Robin_Watts | But the non-pread case is broken. | 17:47.35 |
| rayjj | Robin_Watts: yes, with my fix, the pread case is OK | 17:47.40 |
| Robin_Watts | Right. | 17:47.43 |
| rayjj | agreed | 17:47.44 |
| mvrhel_laptop | rayjj: let me know when you are done having fun with file stuff. I wanted to chat with you about shadings and the clist | 17:47.52 |
| rayjj | runs away | 17:48.10 |
| | ;-) | 17:48.15 |
| mvrhel_laptop | :) | 17:48.15 |
| Robin_Watts | I can't see a way to fix the non pread case, other than by nobbling gp_can_share_stuff to return 0 when we don't have pread. | 17:48.28 |
| rayjj | I think we are done with the file multi-threading issue | 17:48.41 |
| | Robin_Watts: or putting our own lock in place | 17:48.59 |
| Robin_Watts | shuts up and yields the floor to the honourable gentleman from seattle. | 17:49.09 |
| rayjj | Robin_Watts: actually, when we don't share file descriptors, we open the file for read with separate FILE * streams for each thread, so returning 0 ffrom gp_can_share_fdesc should be OK | 17:50.16 |
| Robin_Watts | That's what I've been saying, isn't it? | 17:50.52 |
| | Without pread, we need to return 0 from gp_can_share_fdesc | 17:51.06 |
| rayjj | we assume that if the OS allows multiple FILE *'s open on a file (at least for 'r' fmode) it will work properly for each one | 17:51.21 |
| | Robin_Watts: yes, and we don't have to fix the non pread case because they won't be shared across theads | 17:51.59 |
| Robin_Watts | returning 0 *is* the fix for the non-pread case :) | 17:52.18 |
| rayjj | Robin_Watts: agreed. | 17:52.27 |
| | Ok, mvrhel_laptop, let me have a bio break, and I'll be right back | 17:52.45 |
| mvrhel_laptop | ok | 17:52.49 |
| rayjj | mvrhel_laptop: OK, shoot (small caliber only, please) | 17:55.27 |
| mvrhel_laptop | so I had done a commit a while back http://git.ghostscript.com/?p=ghostpdl.git;a=commit;h=f996deee6d02ed9da7740419ac9fe97a79e6d890 | 17:55.56 |
| | which basically considered cases where the mapping between the source color space and the destination color space was linear | 17:56.25 |
| | and it would avoid the chopping into smaller bits for the shading | 17:56.40 |
| | it came back fine during the regression testing | 17:57.01 |
| | but somehow in the extended testing marcosw found an issue | 17:57.17 |
| | with the file from bug 696227 | 17:57.52 |
| | there is a simple radial gradient fill | 17:58.02 |
| | with a DeviceN color which is only magenta | 17:58.09 |
| | running this at 300dpi with out the clist looks fine | 17:58.25 |
| | running this at 300dpi with the clist drops out the middle bands | 17:58.46 |
| | I am puzzled on how this could be | 17:59.02 |
| | since the trap fills are passed along I would think | 17:59.14 |
| | in either case | 17:59.17 |
| rayjj | mvrhel_laptop: clist bands or radial bands | 17:59.19 |
| mvrhel_laptop | clist bands | 17:59.25 |
| | so the file is a radial gradient centered in the center of the page | 17:59.49 |
| | the middle clist bands are missing | 17:59.55 |
| rayjj | mvrhel_laptop: if you have a simple case, you can use -ZL to look at the clist commands (rather verbose) | 18:00.01 |
| mvrhel_laptop | oh let me look at that | 18:00.12 |
| rayjj | mvrhel_laptop: I suspect that the color is not getting set up properly in the middle bands, but why I have no idea -- just a guess. | 18:01.32 |
| | DeviceN uses high-level color methods to allow for more than fit into the color_index | 18:02.21 |
| mvrhel_laptop | hmm ok let me play around with the clist parameters to see if I can get some useful information from -ZL. there are a lot of trap fills... | 18:04.25 |
| rayjj | mvrhel_laptop: but there are two methods -- clist_fill_trapezoid (for a flat trap or triangle) and clist_fill_linear_color_trapezoid which uses frac31 for the coordinates | 18:06.01 |
| mvrhel_laptop | I see cmd_opv_fill_trapezoid | 18:07.27 |
| | oh clist_fill_linear_color_trapezoid | 18:08.23 |
| | is where I am | 18:08.25 |
| rayjj | mvrhel_laptop: there's a suspicious comment in gxclrast.c:2063 -- this relies on the device NOT returning 0 from the call to the device's fill_linear_color_trapezoid | 18:09.58 |
| mvrhel_laptop | yes just read that | 18:10.06 |
| | and I am looking closer at it | 18:10.14 |
| rayjj | mvrhel_laptop: you may want to set a bp on the gs_note_error (which, AFAIK, doesn't actually error out of the page, but it might stop the band) | 18:11.02 |
| mvrhel_laptop | ok that is not the issue. I ran it in the non-clist case and we never return a zero at that point either | 18:12.53 |
| | i.e. fill_linear_color_trapezoid in the non clist case is returning a non zero always also | 18:13.11 |
| | same resolution | 18:13.16 |
| | and the output is fine | 18:13.21 |
| rayjj | mvrhel_laptop: in the middle bands, is the 'pfs' (patch fill state) getting set up ? | 18:17.57 |
| | oops. I have to run lunches to the kids. bbiaw (about 30 min) -- sorry | 18:18.21 |
| mvrhel_laptop | rayjj: ok no problem | 18:18.29 |
| | I will keep chugging away for a bit | 18:18.36 |
| | this is confusing. there must be something missing. so the colors are there and we are trying to do trap fills in those bands but way down in gx_hl_fill_linear_color_scanline when we never end up doing a rect fill | 18:49.21 |
| | need to eat lunch. this is making my head hurt | 18:52.06 |
| faLUCE | hello, how can I flip vertically 1.pdf, 2.pdf ..... 100.pdf into 1flipped.pdf, 2flipped.pdf .... 100flipped.pdf ? | 19:25.31 |
| rayjj | faLUCE (for the logs): next time wait for more than 7 minutes | 19:55.12 |
| | mvrhel_laptop: I'm back (in case you hadn't noticed). Ping me if you want to work on this | 19:55.51 |
| | Robin_Watts: (et al.): I've pushed a revised patch to my repo for bug 696254 that (1) explains things better and (2) fixes a problem with unix when HAVE_PREAD is false (thanks, Robin) | 19:57.19 |
| | the unix fix was to return false from gp_can_share_fdesc when pread isn't available | 19:58.26 |
| mvrhel_laptop | rayjj: I need to head off to get daughter now. I need to think of an easy way to figure this out. | 21:05.54 |
| | Forward 1 day (to 2015/10/24)>>> | |
IRC Logs