Make ios_system thread-safe

[holzschu]

As requested by @yury and @IMcD23 : ios_system() should be made thread-safe, allowing people to launch commands in multiple tabs.

This is not trivial, since making current_command_root_thread a thread-local variable results in it not being set in most threads, and ios_kill() having no thread ID available to kill.

Ideally, I'd like an implementation that is compatible with current applications.

Making ios_system itself thread-safe will leave the issue of some commands being not thread-safe themselves (2 "python" in separate tabs will almost certainly not work).

[holzschu]

global_errno, which I use to return error codes from the commands, would also have to be made thread-local. That would also create practical issues.

[holzschu]

isMainThread is another variable to consider. It is used when a command starts another command (through "system()" or through pipes): only one command can call coordinateWritingItemAtURL at a time.

I'll need to replace it with another mechanism to make sure each tab has only one "main thread".

[holzschu]

Request for comment:
commit 00d90d1 in the experimental branch contains a potential solution for running ios_system in multiple tabs.

There is a data structure (tabParameters) that stores the information that is specific to a given tab: the thread ID of the main command, the thread ID of the last command on the line (when we pipe multiple commands together), the error number to return, the current directory for this tab (not required for OpenTerm, but useful for Blinkshell), the previous directory for this tab (so cd - works)...

I maintain a NSMutableDictionary of these structures, indexed by the value of stdout. The idea being that stdout is constant for a given tab. When ios_system has to perform an action, it begins by checking the value of stdout and extracting the relevant tabParameters.

In preliminary tests, it seems to be working: ctrl-C in a tab closes the command in that tab, not in the other one. Commands are running in parallel in the different tabs (ping -c 100 google.fr, change tab, wait a bit, go back to tab: the command has terminated).

Systems that don't define stdout, like iVim, are unaffected: there is a single tabParameters, indexed by the value 0.

API changes:

• ios_kill() is replaced with ios_kill(FILE* stream), with stream equal to the value of stdout in the current tab.
• When a tab is definitely closed, we need to call ios_closeTab(FILE* stream) so the data structure associated to this tab is released. Otherwise, the next tab to be opened gets the values for the last closed tab.
• Need to make sure thread_stdout is equal to stdout before calling ios_system() (technically not an API change: that was already true before. It's just more visible now).

@yury, @IMcD23 and @louisdh : do you have comments on this potential change? It's working, but I can still change things based on your input.

(it also outputs a lot of debugging information on screen right now; I'll remove that before the global release).

[yury]

Hi @holzschu

Maybe rename tabParameters to sessionParameters ? It can be splits instead of tabs...

Another thing is NSFileManager. In general it is thread safe. But what if App set delegate for default instance? Maybe it should be new instance? See docs.

[holzschu]

@yury: the renaming is done. About NSFileManager, I'm reading the docs to make sure I do it right.

@louisdh: currently OpenTerm does not start a command while another command is running. Test: start nslookup (interactive mode) in one tab. Move to another tab. type ls. Nothing happens until you move the the first tab and exit nslookup. I think @IMcD23 had written something specifically to do that (but it didn't work at the time with ios_system).

There is also the issue of coordinateWritingItemAtURL: currently, I use it for all commands, even if they don't access a file. If you have 2 sessions in the same directory and start a command in each, coordinateWritingItemAtURL blocks one of them until the other is completed. I'll try to remove it for commands that don't access a file (no redirection, no file access).

[holzschu]

@yury: I have a small issue with auto-complete when we switch tabs: the currentDirectory is not set to that of the current tab until the first command is executed. That confuses auto-complete, which displays the list of files in the previous tab current directory. Is there a command that is execute whenever we swith to another tab?

[yury]

There is no such command in session.

But you can create one and call it from TermController#focus

[holzschu]

Update: commit be8524b (in experimental) creates new instances of NSFileManager as needed and only calls coordinateWritingItemAtURL if the command is likely to change files: if it operates on files or directories, or if its output has been redirected. This is a conservative estimate, but it lets you run multiple instances of ping, for example (not all commands are thread-safe themselves, though. Only the latest release (105b903) of network_ios has a thread-safe ping, and the other commands have not been tested).

Oh, and the multiple debug prints have been removed.

[holzschu]

Update (again): indexing by stdout was a bad idea: a terminal can redefine stdout, for example by duplicating streams.
API change:

• added the function ios_switchSession(void *) which stores current session parameters, indexed by a marker of your choice. Must be called when you switch to a new session, with a marker that is unique to that session. Calling it several times in a row with the same marker has no impact (so it can be called just before ios_system).
• ios_kill() reverts back to the previous version: no parameters, kills the process of the active session (defined by ios_switchSession).

Commit ece3ee1 of https://github.com/holzschu/terminal and 70cc53e of https://github.com/holzschu/blink/tree/thread-utils use the new API.