Running shell commands from an app fails

[humdinger]

This is hrev49522.

I haven't used Ubertuber for some time, and now it crashes. That is, python crashes actually, that's used with the youtube-dl script. See attachment, note the non-UTF8 charcters in line 316.

In Ubertuber I string together a little shell command script that is then executed via system(command->String()); . The failing youtube-dl line is:

youtube-dl --continue --restrict-filenames --no-part --no-cache-dir --format best %URL% ;

Doing the same in Terminal directly works!

On IRC bbjimmy IIRC reported a similar issue with using "ls" from his app. His workaround was to use "ls > /dev/null" (though I don't see how that can work...). Anyway, it doesn't work for me and youtube-dl.

If I comment out the above line that is supposed to download a video clip, there's no crash. Even though there are other invocations of youtube-dl, like

youtube-dl --get-title %URL% 2>&1 | tail -n 1

There are also a whole lot of other script commands involved that don't have a problem, see ​here for example

[humdinger]

crashing python

[humdinger]

As bbjimmy said on IRC, start Ubertuber from Terminal and it works. Just like QupZilla BTW, which doesn't find it's profile when started via double click. Then the paths are messed up: "/boot/apps/QupZilla/"/boot/home"/config/settings/Qt/.config/qupzilla/profiles/default Settings"

Related?

[bbjimmy]

It appears to be an issue caused by a program writing to the standard output device when there is no Terminal to priont to:

a yab script that demonstrates the issue:

#!yab x=system("ls /boot/home") open #1, "/boot/home/output", "w" print #1 x close #1

make the script executable then:

launch from the Terminal ... the output file contains "0", this is the no error indication.

Launch from Tracker and the output file contains "2", some undefined error has occured.

[taos]

It seems that Simutrans is affected, too. Start it from the menu and it won't read its language settings or load saved games, start it from Terminal and everything works as before.

[pulkomandy]

@taos: that may be a different issue: apps running from Terminal are started in the correct directory, while when started from Tracker they are run with /boot/home as the current directory. This is known to confuse many SDL-based apps and other ports. Does our simutrans port already include a patch for this (a typical way to patch this is adding chdir(dirname(argv[0])) in the main function)?

[taos]

Replying to pulkomandy:

Does our simutrans port already include a patch for this (a typical way to patch this is adding chdir(dirname(argv[0])) in the main function)?

I have no idea (there has been a long tradition of beos and/or haiku specific stuff in the simutrans source code so it might be already included). However, it still did work a month ago, when I got my patches for compiling simutrans on PM-haiku upstreamed so that I could remove them from the haikuports recipe. Is there a recent change in haiku that made it necessary to include such a patch?

Simutrans usually should look in the home directory for its own folder with screenshots, savegames, etc. When I last checked if the updated simutrans recipe still produced a working simutrans package it used to work even when started from Tracker. Now, it can't even save a game. I'm not sure if that's because it looks for a directory that's not there (not very probable, it used to just create its own folder in ~) or if it tries to write to a directory that's read-only.

[pulkomandy]

No, this is not a recent change, so if the game was running with the same code before, this is not your problem, and things are probably related to this issue.

It could be the way used to locate the home directory, if getenv("HOME") is used, there will probably be an issue similar to Qupzilla. It seems in the launch daemon environment that variable is not set properly.

[axeld]

While I messed it up originally, $HOME should be set since hrev49487.

[axeld]

I guess that bbjimmy is right, though. Before, everything inherited whatever stdout/stderr was set to in the initial shell. The launch_daemon just doesn't do anything in that regard; IOW they are probably not even set.

[pulkomandy]

Our redirect script sent them to /dev/null, but without that they would get to the syslog and serial port output. Where is/was this set?

As for $HOME, you set it with an extra quoting, which is not escaped by putenv, leading to the problem we see uin Qupzilla. Why not use setenv instead of putenv?

[jackburton]

Replying to pulkomandy:

Our redirect script sent them to /dev/null, but without that they would get to the syslog and serial port output. Where is/was this set?

As for $HOME, you set it with an extra quoting, which is not escaped by putenv, leading to the problem we see uin Qupzilla. Why not use setenv instead of putenv?

Yeah, also because putenv() doesn't copy the passed string: ​http://pubs.opengroup.org/onlinepubs/009695399/functions/putenv.html ​http://www.greenend.org.uk/rjk/tech/putenv.html

[jackburton]

Replying to jackburton:

Yeah, also because putenv() doesn't copy the passed string: ​http://pubs.opengroup.org/onlinepubs/009695399/functions/putenv.html ​http://www.greenend.org.uk/rjk/tech/putenv.html

Replying to myself: it seems our putenv() and setenv() behave the same way: they both copy the passed string. Which is not a good thing in itself, since this could cause problems with applications which expect to be able to modify the string passed to putenv() and have the change reflected to the environment variable.

[taos]

As far as I can tell, hrev49532 solved the problem for Simutrans. Checked with hrev49532 x86_64.

[humdinger]

The issue described in the ticket is still there, Ubertuber gets python to crash. Also, the path to QupZilla's profile is still off, but we're getting closer: /config/settings/Qt/.config/qupzilla/profiles/default . We're missing a "~" or "/boot/home/".

[ttcoder]

@axel in your comment:9 I believe you've pinpointed the heart of the matter [and hat tip to bbjimmy in comment:2 too].. I'm digging into this as this originally just annoyed me for Qupzilla but now I see this issue also prevents our stations from using our "Network Preflet" so it's more important.

NetworkPreflet is a small window that displays network related info. In previous hrevs it worked the same when launched from Terminal or Tracker. But now, e.g. this line

int res = system("grep 'sshd:' /etc/passwd");

will return 0 when run from Terminal, yet it returns 2 when run from Tracker.

So I've modified it like thus:

int res = system("grep 'sshd:' /etc/passwd 2> /boot/home/test");

And the file contains this:

grep: write error: Bad file descriptor

Very telling, no ?

Edit: So in BeOS and older hrevs, everybody had an stdin, stdout, and stderr for sure, either when forked from a terminal's bash or spawned by the registrar, since the registrar was launched by the (bash) master boot script with descriptor 0 1 and 2 leading to dev null "for free". Now we no longer get the dev null on 0 1 and 2 for free and have to do it by hand, if we want to run correctly in Tracker. Probably an easy fix ?

[ttcoder]

Oddities when I do a test with printf():

stdio.h (and the man page) define printf() as returning the number of bytes written or an error code, but it never returns an error here, in Terminal or Tracker.

Also tried to dump stdout (which is a FILE *) using a BAlert, but [it's never NULL], it always has an hex value/address, regardless of context (and the address changes at each invokation for that matter). Dunno if it's normal that FILE * stdout is "valid" in both contexts, in the 'raw' C++ code, yet when said C++ code invokes system() to call a command, said command no longer has an stdout.. Maybe the standard lib's FILE * is just a wrapper around something else anyway.

Beside the stdlib, I also played with POSIX open():

int res  = open("/boot/home/test", O_RDONLY)

and the result is surprising: when run from a Terminal, I get the expected file descriptor value 3, meaning that 0 1 and 2 are already reserved, as expected.

When run from Tracker I get... If you said "0" you're wrong -- I get 10 ! WTH?? :-b

[ttcoder]

ping

Maybe while waiting for a fix my applications can work-around this bug by overriding the value of stdin et alia or something.. In case that's inherited by the forked process or some such ?

[pulkomandy]

I haven't tried it, but normally you would use dup2 for that (and yes, they are inherited through fork): ​http://stackoverflow.com/questions/17518014/using-dup2-to-redirect-input-and-output

[ttcoder]

Thanks pulkomandy!

First some code:

#include <errno.h>
#include <Application.h>
#include <Alert.h>

int main()
{
	BApplication app("application/test");
	
#if 1
	if(write(STDOUT_FILENO, "dummy", 0) < 0)
	{
		// add this work-around for #12298 in applications that rely on ::system() et al, and thus directly or indirectly require calls to write() like this one
		//   write( STDOUT_FILENO, buf, buflen )
		// to *not* return an error even when run from Tracker (where write(stdout) returns -1 with errno 0x80006000):
		
		int devnull_wr = open("/dev/null", O_WRONLY);  // careful to create the new fd _before_ calling close(1)
		close(STDOUT_FILENO);
		dup2(devnull_wr, STDOUT_FILENO);
		close(devnull_wr);
		//+: stderr, maybe also stdin
		
		// write(stdout...) should work from here on
	}
#endif
	
	char buf[100] = "";
	
	sprintf(buf, "fileno(stdout)=%d isatty=%d", fileno(stdout), isatty(STDOUT_FILENO));
	(new BAlert("alert", buf, "ok"))
		->Go();
	
	//int clo = close( STDOUT_FILENO );
	//clo = close( STDOUT_FILENO );  // calling close a second time does return -1 in all cases, so the SECOND call behaves normally
	//sprintf(buf, "close() returns 0x%x", clo);
	//(new BAlert("alert", buf, "ok"))->Go();
	
	errno = 0;
	int checkstdout = write(STDOUT_FILENO, "dummy", 0);  // a 0 length write works just as well
	sprintf(buf, "write() returns %d  errno=0x%x", checkstdout, errno);
	(new BAlert("alert", buf, "ok"))->Go();
	//we get errno 0x80006000 (Bad file descriptor) when run from Tracker indeed (unless the work-around block is activated)
	
	// the "moment of truth" test (emulates what my NetServPreflet does):
	int grep = system("grep 'sshd:' /etc/passwd");
	sprintf(buf, "GREP RETURNS:%d", grep);
	(new BAlert("alert", buf, "ok"))->Go();
	
	return 0;
}

Comments:

Work-around wise, the above if-1-endif block seems to work very well.

Found a weird aspect to this bug though.. And unlike my previous 'findings' that might not be all that relevant, that one does seem wrong: when run from Tracker, close() accepts closing stdout (returns 0) even though calling write() on stdout returns -1, and sets errno to 0x80006000 (Bad file descriptor) So that would be a bug in close() I guess, how can it accept closing a "bad file descriptor"

Also regarding the "10 files already opened" suspicion, found some info that "open always returns lowest unused fd" which would mean that applications launched from Tracker come up with 10 files already opened (maybe inherited from parent after being forked ?). Does that make sense ? Would that constitute a security breach of sorts, in a multi-user environement, for Tracker/Registrar to pass its own file descs to every apps it spawns ? Maybe an FD_CLOEXEC flag would help here

[axeld]

Should be fixed with hrev49630.

[ttcoder]

Still in hrev49627 for a few more hours here but interestingly, QupZilla is fixed there already. It restore its session just the same, whether launched from Terminal or from the Deskbar. So it was likely another problem, unrelated to file descs (assuming others are observing the same).

[axeld]

Yes, QupZilla was victim of the quoted home directory bug.

[jessicah]

Actually, restarting Tracker breaks QupZilla's loading of the profile still. I'm on hrev49642 for reference.