Ubuntu
nfs-utils package

Code review comment for ~ahasenack/ubuntu/+source/nfs-utils:bionic-thread-safety-gssd-1927745

  1. Git
  2. lp:~ahasenack/ubuntu/+source/nfs-utils
  3. bionic-thread-safety-gssd-1927745
  4. Merge into ubuntu/bionic-devel
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

Thanks for the MP, Andreas.

I was able to reproduce the bug locally by following the SRU template. I have a few comments about it:

- I had to resort to "hostnamectl set-hostname foo.example.com" in order to get the FQDN properly working. Maybe it's worth mentioning this there.

- Something I also did while trying to reproduce the bug was to invoke the "./bz1419280_test_threads" in two terminals simultaneously. I don't know if this helps or not, but it seemed to trigger the problem faster.

- LXD will create VMs using only one core by default, it seems. I think it's also worth mentioning that doing "lxc config set vm-name limits.cpu=4" is a good idea before running anything.

Either way, it is obvious that using strtok(3) in a threaded application is a bad idea. I reviewed the patches and they look fine to me.

I double-checked that the package builds fine and the autopkgtest run is still succeeding.

I'm leaving two minor comments in the code, but other than that the MP LGTM. Feel free to proceed after you address them.

review: Approve

« Back to merge proposal